From: 'Christoph Hellwig' <hch@lst.de>
To: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Cc: David Laight <David.Laight@aculab.com>,
'Christoph Hellwig' <hch@lst.de>,
Vlad Yasevich <vyasevich@gmail.com>,
Neil Horman <nhorman@tuxdriver.com>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
"linux-sctp@vger.kernel.org" <linux-sctp@vger.kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>
Subject: Re: do a single memdup_user in sctp_setsockopt
Date: Sat, 23 May 2020 09:19:29 +0200 [thread overview]
Message-ID: <20200523071929.GA10466@lst.de> (raw)
In-Reply-To: <20200522143623.GA386664@localhost.localdomain>
On Fri, May 22, 2020 at 11:36:23AM -0300, Marcelo Ricardo Leitner wrote:
> It's subjective, yes, but we hardly have patches over 5k lines.
> In the case here, as changing the functions also requires changing
> their call later on the file, it helps to be able to check that is was
> properly updated. Ditto for chained functions.
>
> For example, I can spot things like this easier (from
> [PATCH 26/49] sctp: pass a kernel pointer to sctp_setsockopt_auth_key)
>
> @@ -3646,7 +3641,6 @@ static int sctp_setsockopt_auth_key(struct sock *sk,
> }
>
> out:
> - kzfree(authkey);
> return ret;
> }
> ...
> @@ -4771,7 +4765,10 @@ static int sctp_setsockopt(struct sock *sk, int level, int optname,
> }
>
> release_sock(sk);
> - kfree(kopt);
> + if (optname == SCTP_AUTH_KEY)
> + kzfree(kopt);
> + else
> + kfree(kopt);
>
> out_nounlock:
> return retval;
>
> these are 1k lines apart.
>
> Yet, your implementation around this is better:
>
> @@ -3733,7 +3624,7 @@ static int sctp_setsockopt_auth_key(struct sock *sk,
> }
>
> out:
> - kzfree(authkey);
> + memset(authkey, 0, optlen);
> return ret;
> }
>
> so that sctp_setsockopt() doesn't have to handle it specially.
Actually that implementation is wrong, if you want to move to a plain
kfree it would have to be a memzero_explicit.
> What if you two work on a joint patchset for this? The proposals are
> quite close. The differences around the setsockopt handling are
> minimal already. It is basically variable naming, indentation and one
> or another small change like:
I don't really want to waste too much time on this, as what I really
need is to get the kernel_setsockopt removal series in ASAP. I'm happy
to respin this once or twice with clear maintainer guidance (like the
memzero_explicit), but I have no idea what you even meant with your
other example or naming. Tell me what exact changes you want, and
I can do a quick spin, but I don't really want a huge open ended
discussion on how to paint the bikeshed..
Alternatively I'll also happily only do a partial conversion for what
I need for the kernel_setsockopt removal and let you and Dave decided
what you guys prefer for the rest.
next prev parent reply other threads:[~2020-05-23 7:19 UTC|newest]
Thread overview: 58+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-21 17:46 do a single memdup_user in sctp_setsockopt Christoph Hellwig
2020-05-21 17:46 ` [PATCH 01/49] sctp: copy the optval from user space " Christoph Hellwig
2020-05-21 17:46 ` [PATCH 02/49] sctp: pass a kernel pointer to sctp_setsockopt_bindx Christoph Hellwig
2020-05-21 17:46 ` [PATCH 03/49] sctp: pass a kernel pointer to __sctp_setsockopt_connectx Christoph Hellwig
2020-05-21 17:46 ` [PATCH 04/49] sctp: pass a kernel pointer to sctp_setsockopt_disable_fragments Christoph Hellwig
2020-05-21 17:46 ` [PATCH 05/49] sctp: pass a kernel pointer to sctp_setsockopt_events Christoph Hellwig
2020-05-21 17:46 ` [PATCH 06/49] sctp: pass a kernel pointer to sctp_setsockopt_autoclose Christoph Hellwig
2020-05-21 17:46 ` [PATCH 07/49] sctp: pass a kernel pointer to sctp_setsockopt_peer_addr_params Christoph Hellwig
2020-05-21 17:46 ` [PATCH 08/49] sctp: pass a kernel pointer to sctp_setsockopt_delayed_ack Christoph Hellwig
2020-05-21 17:46 ` [PATCH 09/49] sctp: pass a kernel pointer to sctp_setsockopt_partial_delivery_point Christoph Hellwig
2020-05-21 17:46 ` [PATCH 10/49] sctp: pass a kernel pointer to sctp_setsockopt_initmsg Christoph Hellwig
2020-05-21 17:46 ` [PATCH 11/49] sctp: pass a kernel pointer to sctp_setsockopt_default_send_param Christoph Hellwig
2020-05-21 17:46 ` [PATCH 12/49] sctp: pass a kernel pointer to sctp_setsockopt_default_sndinfo Christoph Hellwig
2020-05-21 17:46 ` [PATCH 13/49] sctp: pass a kernel pointer to sctp_setsockopt_primary_addr Christoph Hellwig
2020-05-21 17:46 ` [PATCH 14/49] sctp: pass a kernel pointer to sctp_setsockopt_peer_primary_addr Christoph Hellwig
2020-05-21 17:46 ` [PATCH 15/49] sctp: pass a kernel pointer to sctp_setsockopt_nodelay Christoph Hellwig
2020-05-21 17:46 ` [PATCH 16/49] sctp: pass a kernel pointer to sctp_setsockopt_rtoinfo Christoph Hellwig
2020-05-21 17:46 ` [PATCH 17/49] sctp: pass a kernel pointer to sctp_setsockopt_associnfo Christoph Hellwig
2020-05-21 17:46 ` [PATCH 18/49] sctp: pass a kernel pointer to sctp_setsockopt_mappedv4 Christoph Hellwig
2020-05-21 17:46 ` [PATCH 19/49] sctp: pass a kernel pointer to sctp_setsockopt_maxseg Christoph Hellwig
2020-05-21 17:46 ` [PATCH 20/49] sctp: pass a kernel pointer to sctp_setsockopt_adaptation_layer Christoph Hellwig
2020-05-21 17:46 ` [PATCH 21/49] sctp: pass a kernel pointer to sctp_setsockopt_context Christoph Hellwig
2020-05-21 17:46 ` [PATCH 22/49] sctp: pass a kernel pointer to sctp_setsockopt_fragment_interleave Christoph Hellwig
2020-05-21 17:46 ` [PATCH 23/49] sctp: pass a kernel pointer to sctp_setsockopt_maxburst Christoph Hellwig
2020-05-21 17:46 ` [PATCH 24/49] sctp: pass a kernel pointer to sctp_setsockopt_auth_chunk Christoph Hellwig
2020-05-21 17:47 ` [PATCH 25/49] sctp: pass a kernel pointer to sctp_setsockopt_hmac_ident Christoph Hellwig
2020-05-21 17:47 ` [PATCH 26/49] sctp: pass a kernel pointer to sctp_setsockopt_auth_key Christoph Hellwig
2020-05-21 17:47 ` [PATCH 27/49] sctp: pass a kernel pointer to sctp_setsockopt_active_key Christoph Hellwig
2020-05-21 17:47 ` [PATCH 28/49] sctp: pass a kernel pointer to sctp_setsockopt_del_key Christoph Hellwig
2020-05-21 17:47 ` [PATCH 29/49] sctp: pass a kernel pointer to sctp_setsockopt_deactivate_key Christoph Hellwig
2020-05-21 17:47 ` [PATCH 30/49] sctp: pass a kernel pointer to sctp_setsockopt_auto_asconf Christoph Hellwig
2020-05-21 17:47 ` [PATCH 31/49] sctp: pass a kernel pointer to sctp_setsockopt_paddr_thresholds Christoph Hellwig
2020-05-21 17:47 ` [PATCH 32/49] sctp: pass a kernel pointer to sctp_setsockopt_recvrcvinfo Christoph Hellwig
2020-05-21 17:47 ` [PATCH 33/49] sctp: pass a kernel pointer to sctp_setsockopt_recvnxtinfo Christoph Hellwig
2020-05-21 17:47 ` [PATCH 34/49] sctp: pass a kernel pointer to sctp_setsockopt_pr_supported Christoph Hellwig
2020-05-21 17:47 ` [PATCH 35/49] sctp: pass a kernel pointer to sctp_setsockopt_default_prinfo Christoph Hellwig
2020-05-21 17:47 ` [PATCH 36/49] sctp: pass a kernel pointer to sctp_setsockopt_reconfig_supported Christoph Hellwig
2020-05-21 17:47 ` [PATCH 37/49] sctp: pass a kernel pointer to sctp_setsockopt_enable_strreset Christoph Hellwig
2020-05-21 17:47 ` [PATCH 38/49] sctp: pass a kernel pointer to sctp_setsockopt_reset_streams Christoph Hellwig
2020-05-21 17:47 ` [PATCH 39/49] sctp: pass a kernel pointer to sctp_setsockopt_reset_assoc Christoph Hellwig
2020-05-21 17:47 ` [PATCH 40/49] sctp: pass a kernel pointer to sctp_setsockopt_add_streams Christoph Hellwig
2020-05-21 17:47 ` [PATCH 41/49] sctp: pass a kernel pointer to sctp_setsockopt_scheduler Christoph Hellwig
2020-05-21 17:47 ` [PATCH 42/49] sctp: pass a kernel pointer to sctp_setsockopt_scheduler_value Christoph Hellwig
2020-05-21 17:47 ` [PATCH 43/49] sctp: pass a kernel pointer to sctp_setsockopt_interleaving_supported Christoph Hellwig
2020-05-21 17:47 ` [PATCH 44/49] sctp: pass a kernel pointer to sctp_setsockopt_reuse_port Christoph Hellwig
2020-05-21 17:47 ` [PATCH 45/49] sctp: pass a kernel pointer to sctp_setsockopt_event Christoph Hellwig
2020-05-21 17:47 ` [PATCH 46/49] " Christoph Hellwig
2020-05-21 17:47 ` [PATCH 47/49] sctp: pass a kernel pointer to sctp_setsockopt_auth_supported Christoph Hellwig
2020-05-21 17:47 ` [PATCH 48/49] sctp: pass a kernel pointer to sctp_setsockopt_ecn_supported Christoph Hellwig
2020-05-21 17:47 ` [PATCH 49/49] sctp: pass a kernel pointer to sctp_setsockopt_pf_expose Christoph Hellwig
2020-05-22 8:02 ` do a single memdup_user in sctp_setsockopt David Laight
2020-05-22 14:36 ` Marcelo Ricardo Leitner
2020-05-22 15:52 ` David Laight
2020-05-23 7:19 ` 'Christoph Hellwig' [this message]
2020-05-25 19:37 ` David Laight
2020-05-25 20:59 ` Marcelo Ricardo Leitner
2020-05-25 21:18 ` Marcelo Ricardo Leitner
2020-05-22 23:11 ` David Miller
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=20200523071929.GA10466@lst.de \
--to=hch@lst.de \
--cc=David.Laight@aculab.com \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-sctp@vger.kernel.org \
--cc=marcelo.leitner@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=nhorman@tuxdriver.com \
--cc=vyasevich@gmail.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).