Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

drivers: modem: SARA modem driver leaks sockets #26819

Closed
aport opened this issue Jul 11, 2020 · 4 comments · Fixed by #28972
Closed

drivers: modem: SARA modem driver leaks sockets #26819

aport opened this issue Jul 11, 2020 · 4 comments · Fixed by #28972
Assignees
Labels
area: Modem bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale

Comments

@aport
Copy link
Contributor

aport commented Jul 11, 2020

Describe the bug
When calling close on a socket, the ublox sara driver only issues the +USOCL command if the socket is UDP, or if it's TCP and connected.

I am porting existing POSIX software to Zephyr. This code calls close() on a socket descriptor if connect() fails. The result on Zephyr is that the sockets allocated internally by the module are never released. On Linux the file descriptor will be released.

To Reproduce
Steps to reproduce the behavior:

  1. Use the SARA-R4 driver
  2. Create a TCP socket with socket(AF_INET, SOCK_STREAM, IPPROTO_TCP)
  3. Note the ID of the socket (modem logging helps)
  4. Call close on the socket
  5. See that the socket is not released, no +USOCL is issued to the module
  6. Repeat step 2
  7. See that a new, different socket is allocated by the module. There are now two existing sockets inside the SARA.

Expected behavior
Calling close on a socket should release its resources, independent on whether it is connected or not.

Impact
Eventually the module runs out of internal sockets and new socket calls fail. The R4 only supports 7 sockets.

Environment (please complete the following information):

  • OS: Linux
  • Toolchain gnuarmemb
  • a18a73c (master as of the creation of this ticket)
@aport aport added the bug The issue is a bug, or the PR is fixing a bug label Jul 11, 2020
@MaureenHelm MaureenHelm added the priority: medium Medium impact/importance bug label Jul 21, 2020
@carlescufi
Copy link
Member

@aport did you work around this or find a fix? if so a Pull Request would be appreciated since there's been no activity on this issue.
@mike-scott any chance you'll be able to look at this?

@carlescufi carlescufi added priority: low Low impact/importance bug and removed priority: medium Medium impact/importance bug labels Sep 8, 2020
@mike-scott
Copy link
Contributor

@carlescufi I have not looked at this. @aport a PR would be greatly appreciated.

@aport
Copy link
Contributor Author

aport commented Sep 9, 2020

Hi, sorry I have not set up a pull request. My tree is a bit out of date and I've been sidetracked by other projects.

My change was minimal, simply the removal of the conditional at https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/modem/ublox-sara-r4.c#L1219 so that the USOCL is always sent, letting the modem figure out if it's valid or not.

When I get some time I'll try to do a PR for this

peltsu added a commit to peltsu/zephyr that referenced this issue Oct 21, 2020
z_free_fd() is called twice then you close(). For example in ublox sara
r4 driver offload_close() calls modem_socket_put() where z_free_fd() is
called first time. Then this same function is called another time in
socket.c in z_impl_zsock_close() after this function has called
offload_close() in ublox sara r4 driver. This causes socket
descriptor leak.

Fixes zephyrproject-rtos#26819

Signed-off-by: Akseli Peltola <[email protected]>
@github-actions
Copy link

github-actions bot commented Nov 9, 2020

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

@github-actions github-actions bot added the Stale label Nov 9, 2020
jukkar pushed a commit that referenced this issue Jan 7, 2021
z_free_fd() is called twice then you close(). For example in ublox sara
r4 driver offload_close() calls modem_socket_put() where z_free_fd() is
called first time. Then this same function is called another time in
socket.c in z_impl_zsock_close() after this function has called
offload_close() in ublox sara r4 driver. This causes socket
descriptor leak.

Fixes #26819

Signed-off-by: Akseli Peltola <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Modem bug The issue is a bug, or the PR is fixing a bug priority: low Low impact/importance bug Stale
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants