Skip to content

Commit

Permalink
net: sockets: Cleanup socket properly if POSIX API is enabled
Browse files Browse the repository at this point in the history
The sock_obj_core_dealloc() was not called if close() is called
instead of zsock_close(). This happens if POSIX API is enabled.

Fix this by calling zvfs_close() from zsock_close() and then
pass the socket number to zsock_close_ctx() so that the cleanup
can be done properly.

Reported-by: Andreas Ålgård <aal@ixys.no>
Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
  • Loading branch information
jukkar committed Oct 28, 2024
1 parent 9f93ded commit fe7b1f9
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 40 deletions.
5 changes: 4 additions & 1 deletion include/zephyr/sys/fdtable.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ struct fd_op_vtable {
ssize_t (*write)(void *obj, const void *buf, size_t sz);
ssize_t (*write_offs)(void *obj, const void *buf, size_t sz, size_t offset);
};
int (*close)(void *obj);
union {
int (*close)(void *obj);
int (*close2)(void *obj, int fd);
};
int (*ioctl)(void *obj, unsigned int request, va_list args);
};

Expand Down
9 changes: 8 additions & 1 deletion lib/os/fdtable.c
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,14 @@ int zvfs_close(int fd)
(void)k_mutex_lock(&fdtable[fd].lock, K_FOREVER);
if (fdtable[fd].vtable->close != NULL) {
/* close() is optional - e.g. stdinout_fd_op_vtable */
res = fdtable[fd].vtable->close(fdtable[fd].obj);
if (fdtable[fd].mode & ZVFS_MODE_IFSOCK) {
/* Network socket needs to know socket number so pass
* it via close2() call.
*/
res = fdtable[fd].vtable->close2(fdtable[fd].obj, fd);
} else {
res = fdtable[fd].vtable->close(fdtable[fd].obj);
}
}
k_mutex_unlock(&fdtable[fd].lock);

Expand Down
32 changes: 8 additions & 24 deletions subsys/net/lib/sockets/sockets.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,42 +147,26 @@ static inline int z_vrfy_zsock_socket(int family, int type, int proto)
#include <zephyr/syscalls/zsock_socket_mrsh.c>
#endif /* CONFIG_USERSPACE */

extern int zvfs_close(int fd);

int z_impl_zsock_close(int sock)
{
return zvfs_close(sock);
}

#ifdef CONFIG_USERSPACE
static inline int z_vrfy_zsock_close(int sock)
{
const struct socket_op_vtable *vtable;
struct k_mutex *lock;
void *ctx;
int ret;

SYS_PORT_TRACING_OBJ_FUNC_ENTER(socket, close, sock);

ctx = get_sock_vtable(sock, &vtable, &lock);
if (ctx == NULL) {
errno = EBADF;
SYS_PORT_TRACING_OBJ_FUNC_EXIT(socket, close, sock, -errno);
return -1;
}

(void)k_mutex_lock(lock, K_FOREVER);

NET_DBG("close: ctx=%p, fd=%d", ctx, sock);

ret = vtable->fd_vtable.close(ctx);

k_mutex_unlock(lock);

SYS_PORT_TRACING_OBJ_FUNC_EXIT(socket, close, sock, ret < 0 ? -errno : ret);

zvfs_free_fd(sock);

(void)sock_obj_core_dealloc(sock);

return ret;
}

#ifdef CONFIG_USERSPACE
static inline int z_vrfy_zsock_close(int sock)
{
return z_impl_zsock_close(sock);
}
#include <zephyr/syscalls/zsock_close_mrsh.c>
Expand Down
22 changes: 16 additions & 6 deletions subsys/net/lib/sockets/sockets_inet.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,14 @@ static int zsock_socket_internal(int family, int type, int proto)
return fd;
}

int zsock_close_ctx(struct net_context *ctx)
int zsock_close_ctx(struct net_context *ctx, int sock)
{
int ret;

SYS_PORT_TRACING_OBJ_FUNC_ENTER(socket, close, sock);

NET_DBG("close: ctx=%p, fd=%d", ctx, sock);

/* Reset callbacks to avoid any race conditions while
* flushing queues. No need to check return values here,
* as these are fail-free operations and we're closing
Expand All @@ -160,10 +164,16 @@ int zsock_close_ctx(struct net_context *ctx)
ret = net_context_put(ctx);
if (ret < 0) {
errno = -ret;
return -1;
ret = -1;
}

return 0;
SYS_PORT_TRACING_OBJ_FUNC_EXIT(socket, close, sock, ret < 0 ? -errno : ret);

if (ret == 0) {
(void)sock_obj_core_dealloc(sock);
}

return ret;
}

static void zsock_accepted_cb(struct net_context *new_ctx,
Expand Down Expand Up @@ -2771,9 +2781,9 @@ static int sock_setsockopt_vmeth(void *obj, int level, int optname,
return zsock_setsockopt_ctx(obj, level, optname, optval, optlen);
}

static int sock_close_vmeth(void *obj)
static int sock_close2_vmeth(void *obj, int fd)
{
return zsock_close_ctx(obj);
return zsock_close_ctx(obj, fd);
}
static int sock_getpeername_vmeth(void *obj, struct sockaddr *addr,
socklen_t *addrlen)
Expand All @@ -2791,7 +2801,7 @@ const struct socket_op_vtable sock_fd_op_vtable = {
.fd_vtable = {
.read = sock_read_vmeth,
.write = sock_write_vmeth,
.close = sock_close_vmeth,
.close2 = sock_close2_vmeth,
.ioctl = sock_ioctl_vmeth,
},
.shutdown = sock_shutdown_vmeth,

Check notice on line 2807 in subsys/net/lib/sockets/sockets_inet.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/net/lib/sockets/sockets_inet.c:2807 - .fd_vtable = { - .read = sock_read_vmeth, - .write = sock_write_vmeth, - .close2 = sock_close2_vmeth, - .ioctl = sock_ioctl_vmeth, - }, + .fd_vtable = + { + .read = sock_read_vmeth, + .write = sock_write_vmeth, + .close2 = sock_close2_vmeth, + .ioctl = sock_ioctl_vmeth, + },

Check notice on line 2807 in subsys/net/lib/sockets/sockets_inet.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/net/lib/sockets/sockets_inet.c:2807 - .fd_vtable = { - .read = sock_read_vmeth, - .write = sock_write_vmeth, - .close2 = sock_close2_vmeth, - .ioctl = sock_ioctl_vmeth, - }, + .fd_vtable = + { + .read = sock_read_vmeth, + .write = sock_write_vmeth, + .close2 = sock_close2_vmeth, + .ioctl = sock_ioctl_vmeth, + },
Expand Down
2 changes: 1 addition & 1 deletion subsys/net/lib/sockets/sockets_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#define SOCK_NONBLOCK 2
#define SOCK_ERROR 4

int zsock_close_ctx(struct net_context *ctx);
int zsock_close_ctx(struct net_context *ctx, int sock);
int zsock_poll_internal(struct zsock_pollfd *fds, int nfds, k_timeout_t timeout);

int zsock_wait_data(struct net_context *ctx, k_timeout_t *timeout);
Expand Down
6 changes: 3 additions & 3 deletions subsys/net/lib/sockets/sockets_packet.c
Original file line number Diff line number Diff line change
Expand Up @@ -462,16 +462,16 @@ static int packet_sock_setsockopt_vmeth(void *obj, int level, int optname,
return zpacket_setsockopt_ctx(obj, level, optname, optval, optlen);
}

static int packet_sock_close_vmeth(void *obj)
static int packet_sock_close2_vmeth(void *obj, int fd)
{
return zsock_close_ctx(obj);
return zsock_close_ctx(obj, fd);
}

static const struct socket_op_vtable packet_sock_fd_op_vtable = {
.fd_vtable = {
.read = packet_sock_read_vmeth,
.write = packet_sock_write_vmeth,
.close = packet_sock_close_vmeth,
.close2 = packet_sock_close2_vmeth,
.ioctl = packet_sock_ioctl_vmeth,
},
.bind = packet_sock_bind_vmeth,

Check notice on line 477 in subsys/net/lib/sockets/sockets_packet.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/net/lib/sockets/sockets_packet.c:477 - .fd_vtable = { - .read = packet_sock_read_vmeth, - .write = packet_sock_write_vmeth, - .close2 = packet_sock_close2_vmeth, - .ioctl = packet_sock_ioctl_vmeth, - }, + .fd_vtable = + { + .read = packet_sock_read_vmeth, + .write = packet_sock_write_vmeth, + .close2 = packet_sock_close2_vmeth, + .ioctl = packet_sock_ioctl_vmeth, + },

Check notice on line 477 in subsys/net/lib/sockets/sockets_packet.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/net/lib/sockets/sockets_packet.c:477 - .fd_vtable = { - .read = packet_sock_read_vmeth, - .write = packet_sock_write_vmeth, - .close2 = packet_sock_close2_vmeth, - .ioctl = packet_sock_ioctl_vmeth, - }, + .fd_vtable = + { + .read = packet_sock_read_vmeth, + .write = packet_sock_write_vmeth, + .close2 = packet_sock_close2_vmeth, + .ioctl = packet_sock_ioctl_vmeth, + },
Expand Down
12 changes: 8 additions & 4 deletions subsys/net/lib/sockets/sockets_tls.c
Original file line number Diff line number Diff line change
Expand Up @@ -2108,7 +2108,7 @@ static int ztls_socket(int family, int type, int proto)
return -1;
}

int ztls_close_ctx(struct tls_context *ctx)
int ztls_close_ctx(struct tls_context *ctx, int sock)
{
int ret, err = 0;

Expand All @@ -2120,6 +2120,10 @@ int ztls_close_ctx(struct tls_context *ctx)
err = tls_release(ctx);
ret = zsock_close(ctx->sock);

if (ret == 0) {
(void)sock_obj_core_dealloc(sock);
}

/* In case close fails, we propagate errno value set by close.
* In case close succeeds, but tls_release fails, set errno
* according to tls_release return value.
Expand Down Expand Up @@ -3826,9 +3830,9 @@ static int tls_sock_setsockopt_vmeth(void *obj, int level, int optname,
return ztls_setsockopt_ctx(obj, level, optname, optval, optlen);
}

static int tls_sock_close_vmeth(void *obj)
static int tls_sock_close2_vmeth(void *obj, int sock)
{
return ztls_close_ctx(obj);
return ztls_close_ctx(obj, sock);
}

static int tls_sock_getpeername_vmeth(void *obj, struct sockaddr *addr,
Expand All @@ -3851,7 +3855,7 @@ static const struct socket_op_vtable tls_sock_fd_op_vtable = {
.fd_vtable = {
.read = tls_sock_read_vmeth,
.write = tls_sock_write_vmeth,
.close = tls_sock_close_vmeth,
.close2 = tls_sock_close2_vmeth,
.ioctl = tls_sock_ioctl_vmeth,
},
.shutdown = tls_sock_shutdown_vmeth,

Check notice on line 3861 in subsys/net/lib/sockets/sockets_tls.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/net/lib/sockets/sockets_tls.c:3861 - .fd_vtable = { - .read = tls_sock_read_vmeth, - .write = tls_sock_write_vmeth, - .close2 = tls_sock_close2_vmeth, - .ioctl = tls_sock_ioctl_vmeth, - }, + .fd_vtable = + { + .read = tls_sock_read_vmeth, + .write = tls_sock_write_vmeth, + .close2 = tls_sock_close2_vmeth, + .ioctl = tls_sock_ioctl_vmeth, + },

Check notice on line 3861 in subsys/net/lib/sockets/sockets_tls.c

View workflow job for this annotation

GitHub Actions / Run compliance checks on patch series (PR)

You may want to run clang-format on this change

subsys/net/lib/sockets/sockets_tls.c:3861 - .fd_vtable = { - .read = tls_sock_read_vmeth, - .write = tls_sock_write_vmeth, - .close2 = tls_sock_close2_vmeth, - .ioctl = tls_sock_ioctl_vmeth, - }, + .fd_vtable = + { + .read = tls_sock_read_vmeth, + .write = tls_sock_write_vmeth, + .close2 = tls_sock_close2_vmeth, + .ioctl = tls_sock_ioctl_vmeth, + },
Expand Down

0 comments on commit fe7b1f9

Please sign in to comment.