linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: anant.thazhemadam@gmail.com
Cc: syzbot+09a5d591c1f98cf5efcb@syzkaller.appspotmail.com,
	netdev@vger.kernel.org,
	linux-kernel-mentees@lists.linuxfoundation.org,
	linux-kernel@vger.kernel.org, kuba@kernel.org
Subject: Re: [Linux-kernel-mentees] [PATCH v2] net: fix uninit value error in __sys_sendmmsg
Date: Sun, 13 Sep 2020 14:39:39 -0700 (PDT)	[thread overview]
Message-ID: <20200913.143939.859765790019703223.davem@davemloft.net> (raw)
In-Reply-To: <20200913110313.4239-1-anant.thazhemadam@gmail.com>

From: Anant Thazhemadam <anant.thazhemadam@gmail.com>
Date: Sun, 13 Sep 2020 16:33:13 +0530

> diff --git a/net/socket.c b/net/socket.c
> index 0c0144604f81..1e6f9b54982c 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -2398,6 +2398,7 @@ static int ___sys_sendmsg(struct socket *sock, struct user_msghdr __user *msg,
>  	struct iovec iovstack[UIO_FASTIOV], *iov = iovstack;
>  	ssize_t err;
>  
> +	memset(iov, 0, UIO_FASTIOV);
>  	msg_sys->msg_name = &address;

Did you even test this?

Seriously?

UIO_FASTIOV is the number of entries in 'iovstack', it's not the
size with would be "UIO_FASTIOV * sizeof (struct iovec)", or
even "sizeof(iovstack)"

So could you really explain to me how you tested this patch for
correctness, and for any functional or performance regressions
that may occur?

Because, once you correct that size argument to memset() we will now
have a huge memset() for _EVERY_ _SINGLE_ sendmsg() done by the
system.  And that will cause severe performance regressions for many
workloads involving networking.

This patch submission has been extremely careless on so many levels. I
sincerely wish you would take your time with these changes and not be
so lacking in the areas of testing and validation.

It is always a reg flag when a submitter doesn't even notice an
obvious compiler warning that reviewers like Greg and myself can see
even without trying to build your code changes.

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

  reply	other threads:[~2020-09-13 21:39 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-13 11:03 [Linux-kernel-mentees] [PATCH v2] net: fix uninit value error in __sys_sendmmsg Anant Thazhemadam
2020-09-13 21:39 ` David Miller [this message]
2020-09-14  8:14 ` Anant Thazhemadam

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200913.143939.859765790019703223.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=anant.thazhemadam@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel-mentees@lists.linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=syzbot+09a5d591c1f98cf5efcb@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).