* [PATCH 0/2] cifs: don't send uninitialized memory to sock_{send,recv}msg() @ 2022-09-14 3:25 Stefan Metzmacher 2022-09-14 3:25 ` [PATCH 1/2] cifs: don't send down the destination address to sendmsg for a SOCK_STREAM Stefan Metzmacher ` (2 more replies) 0 siblings, 3 replies; 4+ messages in thread From: Stefan Metzmacher @ 2022-09-14 3:25 UTC (permalink / raw) To: linux-cifs; +Cc: Stefan Metzmacher Passing just half initialized struct msghdr variables down to sock_{send,recv}msg() means we're waiting for a disater to happen... I added the removal of passing the destination address to tcp as a separate patch in order to explain it separately. Stefan Metzmacher (2): cifs: don't send down the destination address to sendmsg for a SOCK_STREAM cifs: always initialize struct msghdr smb_msg completely fs/cifs/connect.c | 11 +++-------- fs/cifs/transport.c | 6 +----- 2 files changed, 4 insertions(+), 13 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] cifs: don't send down the destination address to sendmsg for a SOCK_STREAM 2022-09-14 3:25 [PATCH 0/2] cifs: don't send uninitialized memory to sock_{send,recv}msg() Stefan Metzmacher @ 2022-09-14 3:25 ` Stefan Metzmacher 2022-09-14 3:25 ` [PATCH 2/2] cifs: always initialize struct msghdr smb_msg completely Stefan Metzmacher 2022-09-14 3:46 ` [PATCH 0/2] cifs: don't send uninitialized memory to sock_{send,recv}msg() ronnie sahlberg 2 siblings, 0 replies; 4+ messages in thread From: Stefan Metzmacher @ 2022-09-14 3:25 UTC (permalink / raw) To: linux-cifs; +Cc: Stefan Metzmacher, stable, Paulo Alcantara This is ignored anyway by the tcp layer. Signed-off-by: Stefan Metzmacher <metze@samba.org> Cc: stable@vger.kernel.org Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> --- fs/cifs/transport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index c2fe035e573b..a43c87c1d343 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -194,8 +194,8 @@ smb_send_kvec(struct TCP_Server_Info *server, struct msghdr *smb_msg, *sent = 0; - smb_msg->msg_name = (struct sockaddr *) &server->dstaddr; - smb_msg->msg_namelen = sizeof(struct sockaddr); + smb_msg->msg_name = NULL; + smb_msg->msg_namelen = 0; smb_msg->msg_control = NULL; smb_msg->msg_controllen = 0; if (server->noblocksnd) -- 2.34.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] cifs: always initialize struct msghdr smb_msg completely 2022-09-14 3:25 [PATCH 0/2] cifs: don't send uninitialized memory to sock_{send,recv}msg() Stefan Metzmacher 2022-09-14 3:25 ` [PATCH 1/2] cifs: don't send down the destination address to sendmsg for a SOCK_STREAM Stefan Metzmacher @ 2022-09-14 3:25 ` Stefan Metzmacher 2022-09-14 3:46 ` [PATCH 0/2] cifs: don't send uninitialized memory to sock_{send,recv}msg() ronnie sahlberg 2 siblings, 0 replies; 4+ messages in thread From: Stefan Metzmacher @ 2022-09-14 3:25 UTC (permalink / raw) To: linux-cifs; +Cc: Stefan Metzmacher, stable, Paulo Alcantara So far we were just lucky because the uninitialized members of struct msghdr are not used by default on a SOCK_STREAM tcp socket. But as new things like msg_ubuf and sg_from_iter where added recently, we should play on the safe side and avoid potention problems in future. Signed-off-by: Stefan Metzmacher <metze@samba.org> Cc: stable@vger.kernel.org Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> --- fs/cifs/connect.c | 11 +++-------- fs/cifs/transport.c | 6 +----- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index a0a06b6f252b..0225f4c8adf0 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -702,9 +702,6 @@ cifs_readv_from_socket(struct TCP_Server_Info *server, struct msghdr *smb_msg) int length = 0; int total_read; - smb_msg->msg_control = NULL; - smb_msg->msg_controllen = 0; - for (total_read = 0; msg_data_left(smb_msg); total_read += length) { try_to_freeze(); @@ -760,7 +757,7 @@ int cifs_read_from_socket(struct TCP_Server_Info *server, char *buf, unsigned int to_read) { - struct msghdr smb_msg; + struct msghdr smb_msg = {}; struct kvec iov = {.iov_base = buf, .iov_len = to_read}; iov_iter_kvec(&smb_msg.msg_iter, READ, &iov, 1, to_read); @@ -770,15 +767,13 @@ cifs_read_from_socket(struct TCP_Server_Info *server, char *buf, ssize_t cifs_discard_from_socket(struct TCP_Server_Info *server, size_t to_read) { - struct msghdr smb_msg; + struct msghdr smb_msg = {}; /* * iov_iter_discard already sets smb_msg.type and count and iov_offset * and cifs_readv_from_socket sets msg_control and msg_controllen * so little to initialize in struct msghdr */ - smb_msg.msg_name = NULL; - smb_msg.msg_namelen = 0; iov_iter_discard(&smb_msg.msg_iter, READ, to_read); return cifs_readv_from_socket(server, &smb_msg); @@ -788,7 +783,7 @@ int cifs_read_page_from_socket(struct TCP_Server_Info *server, struct page *page, unsigned int page_offset, unsigned int to_read) { - struct msghdr smb_msg; + struct msghdr smb_msg = {}; struct bio_vec bv = { .bv_page = page, .bv_len = to_read, .bv_offset = page_offset}; iov_iter_bvec(&smb_msg.msg_iter, READ, &bv, 1, to_read); diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c index a43c87c1d343..9a2753e21170 100644 --- a/fs/cifs/transport.c +++ b/fs/cifs/transport.c @@ -194,10 +194,6 @@ smb_send_kvec(struct TCP_Server_Info *server, struct msghdr *smb_msg, *sent = 0; - smb_msg->msg_name = NULL; - smb_msg->msg_namelen = 0; - smb_msg->msg_control = NULL; - smb_msg->msg_controllen = 0; if (server->noblocksnd) smb_msg->msg_flags = MSG_DONTWAIT + MSG_NOSIGNAL; else @@ -309,7 +305,7 @@ __smb_send_rqst(struct TCP_Server_Info *server, int num_rqst, sigset_t mask, oldmask; size_t total_len = 0, sent, size; struct socket *ssocket = server->ssocket; - struct msghdr smb_msg; + struct msghdr smb_msg = {}; __be32 rfc1002_marker; if (cifs_rdma_enabled(server)) { -- 2.34.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 0/2] cifs: don't send uninitialized memory to sock_{send,recv}msg() 2022-09-14 3:25 [PATCH 0/2] cifs: don't send uninitialized memory to sock_{send,recv}msg() Stefan Metzmacher 2022-09-14 3:25 ` [PATCH 1/2] cifs: don't send down the destination address to sendmsg for a SOCK_STREAM Stefan Metzmacher 2022-09-14 3:25 ` [PATCH 2/2] cifs: always initialize struct msghdr smb_msg completely Stefan Metzmacher @ 2022-09-14 3:46 ` ronnie sahlberg 2 siblings, 0 replies; 4+ messages in thread From: ronnie sahlberg @ 2022-09-14 3:46 UTC (permalink / raw) To: Stefan Metzmacher; +Cc: linux-cifs Reviewed by me for both patches On Wed, 14 Sept 2022 at 13:34, Stefan Metzmacher <metze@samba.org> wrote: > > Passing just half initialized struct msghdr variables down to > sock_{send,recv}msg() means we're waiting for a disater to happen... > > I added the removal of passing the destination address to > tcp as a separate patch in order to explain it separately. > > Stefan Metzmacher (2): > cifs: don't send down the destination address to sendmsg for a > SOCK_STREAM > cifs: always initialize struct msghdr smb_msg completely > > fs/cifs/connect.c | 11 +++-------- > fs/cifs/transport.c | 6 +----- > 2 files changed, 4 insertions(+), 13 deletions(-) > > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-09-14 3:46 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-09-14 3:25 [PATCH 0/2] cifs: don't send uninitialized memory to sock_{send,recv}msg() Stefan Metzmacher 2022-09-14 3:25 ` [PATCH 1/2] cifs: don't send down the destination address to sendmsg for a SOCK_STREAM Stefan Metzmacher 2022-09-14 3:25 ` [PATCH 2/2] cifs: always initialize struct msghdr smb_msg completely Stefan Metzmacher 2022-09-14 3:46 ` [PATCH 0/2] cifs: don't send uninitialized memory to sock_{send,recv}msg() ronnie sahlberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).