* [PATCH 0/2] sctp: socket cleanups @ 2011-05-12 19:19 Joe Perches 2011-05-12 19:19 ` [PATCH 1/2] sctp: sctp_sendmsg: Don't initialize default_sinfo Joe Perches 2011-05-12 19:19 ` [PATCH 2/2] sctp: sctp_sendmsg: Don't test known non-null sinfo Joe Perches 0 siblings, 2 replies; 7+ messages in thread From: Joe Perches @ 2011-05-12 19:19 UTC (permalink / raw) To: linux-sctp; +Cc: netdev, linux-kernel Size trivially reduced as well. $ size net/sctp/socket.o text data bss dec hex filename 55720 442 14988 71150 115ee net/sctp/socket.o.new 55730 442 14996 71168 11600 net/sctp/socket.o.old Joe Perches (2): sctp: sctp_sendmsg: Don't initialize default_sinfo sctp: sctp_sendmsg: Don't test known non-null sinfo net/sctp/socket.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-) -- 1.7.5.rc3.dirty ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] sctp: sctp_sendmsg: Don't initialize default_sinfo 2011-05-12 19:19 [PATCH 0/2] sctp: socket cleanups Joe Perches @ 2011-05-12 19:19 ` Joe Perches 2011-05-12 21:06 ` David Miller 2011-05-12 19:19 ` [PATCH 2/2] sctp: sctp_sendmsg: Don't test known non-null sinfo Joe Perches 1 sibling, 1 reply; 7+ messages in thread From: Joe Perches @ 2011-05-12 19:19 UTC (permalink / raw) To: Vlad Yasevich, Sridhar Samudrala Cc: David S. Miller, linux-sctp, netdev, linux-kernel This variable only needs initialization when cmsgs.info is NULL. Don't use memset, just initialize every struct member. Signed-off-by: Joe Perches <joe@perches.com> --- net/sctp/socket.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 33d9ee6..a5d3560 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1496,8 +1496,8 @@ SCTP_STATIC int sctp_sendmsg(struct kiocb *iocb, struct sock *sk, struct sctp_chunk *chunk; union sctp_addr to; struct sockaddr *msg_name = NULL; - struct sctp_sndrcvinfo default_sinfo = { 0 }; struct sctp_sndrcvinfo *sinfo; + struct sctp_sndrcvinfo default_sinfo; struct sctp_initmsg *sinit; sctp_assoc_t associd = 0; sctp_cmsgs_t cmsgs = { NULL }; @@ -1761,10 +1761,13 @@ SCTP_STATIC int sctp_sendmsg(struct kiocb *iocb, struct sock *sk, * some defaults. */ default_sinfo.sinfo_stream = asoc->default_stream; + default_sinfo.sinfo_ssn = 0; default_sinfo.sinfo_flags = asoc->default_flags; default_sinfo.sinfo_ppid = asoc->default_ppid; default_sinfo.sinfo_context = asoc->default_context; default_sinfo.sinfo_timetolive = asoc->default_timetolive; + default_sinfo.sinfo_tsn = 0; + default_sinfo.sinfo_cumtsn = 0; default_sinfo.sinfo_assoc_id = sctp_assoc2id(asoc); sinfo = &default_sinfo; } -- 1.7.5.rc3.dirty ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] sctp: sctp_sendmsg: Don't initialize default_sinfo 2011-05-12 19:19 ` [PATCH 1/2] sctp: sctp_sendmsg: Don't initialize default_sinfo Joe Perches @ 2011-05-12 21:06 ` David Miller 2011-05-12 21:27 ` [PATCH 1/2 v2] " Joe Perches 0 siblings, 1 reply; 7+ messages in thread From: David Miller @ 2011-05-12 21:06 UTC (permalink / raw) To: joe; +Cc: vladislav.yasevich, sri, linux-sctp, netdev, linux-kernel From: Joe Perches <joe@perches.com> Date: Thu, 12 May 2011 12:19:09 -0700 > This variable only needs initialization when cmsgs.info > is NULL. > > Don't use memset, just initialize every struct member. > > Signed-off-by: Joe Perches <joe@perches.com> I don't think you do this, this structure has padding holes on pretty much every architecture. It starts with 3 u16's, then there is a u32, so there is a 2-byte piece of padding after the 3rd u16. Can you prove that these uninitialized portions never make it to userspace? If you can, that proof belongs in the commit message. I think it's too risky. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2 v2] sctp: sctp_sendmsg: Don't initialize default_sinfo 2011-05-12 21:06 ` David Miller @ 2011-05-12 21:27 ` Joe Perches 2011-05-12 21:31 ` David Miller 0 siblings, 1 reply; 7+ messages in thread From: Joe Perches @ 2011-05-12 21:27 UTC (permalink / raw) To: David Miller; +Cc: vladislav.yasevich, sri, linux-sctp, netdev, linux-kernel This variable only needs initialization when cmsgs.info is NULL. Use memset to ensure padding is also zeroed so kernel doesn't leak any data. Signed-off-by: Joe Perches <joe@perches.com> --- On Thu, 2011-05-12 at 17:06 -0400, David Miller wrote: From: Joe Perches <joe@perches.com> > Date: Thu, 12 May 2011 12:19:09 -0700 > > This variable only needs initialization when cmsgs.info > > is NULL. > > Don't use memset, just initialize every struct member. > > Signed-off-by: Joe Perches <joe@perches.com> > I don't think you do this, this structure has padding holes on pretty > much every architecture. > It starts with 3 u16's, then there is a u32, so there is a 2-byte > piece of padding after the 3rd u16. > Can you prove that these uninitialized portions never make it to > userspace? If you can, that proof belongs in the commit message. Thanks David. I didn't notice it went to userspace. > I think it's too risky. It is. I like memset. The current initialization isn't guaranteed by c90 standard to zero all padding either. In practice it does though. The idea was to avoid doing a (non-memset) struct foo bar = {} when unnecessary for every packet as it's only needed when cmsgs.info is NULL. net/sctp/socket.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 33d9ee6..d4b8db1 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1496,7 +1496,7 @@ SCTP_STATIC int sctp_sendmsg(struct kiocb *iocb, struct sock *sk, struct sctp_chunk *chunk; union sctp_addr to; struct sockaddr *msg_name = NULL; - struct sctp_sndrcvinfo default_sinfo = { 0 }; + struct sctp_sndrcvinfo default_sinfo; struct sctp_sndrcvinfo *sinfo; struct sctp_initmsg *sinit; sctp_assoc_t associd = 0; @@ -1760,6 +1760,7 @@ SCTP_STATIC int sctp_sendmsg(struct kiocb *iocb, struct sock *sk, /* If the user didn't specify SNDRCVINFO, make up one with * some defaults. */ + memset(&default_sinfo, 0, sizeof(default_sinfo)); default_sinfo.sinfo_stream = asoc->default_stream; default_sinfo.sinfo_flags = asoc->default_flags; default_sinfo.sinfo_ppid = asoc->default_ppid; ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2 v2] sctp: sctp_sendmsg: Don't initialize default_sinfo 2011-05-12 21:27 ` [PATCH 1/2 v2] " Joe Perches @ 2011-05-12 21:31 ` David Miller 0 siblings, 0 replies; 7+ messages in thread From: David Miller @ 2011-05-12 21:31 UTC (permalink / raw) To: joe; +Cc: vladislav.yasevich, sri, linux-sctp, netdev, linux-kernel From: Joe Perches <joe@perches.com> Date: Thu, 12 May 2011 14:27:20 -0700 > This variable only needs initialization when cmsgs.info > is NULL. > > Use memset to ensure padding is also zeroed so > kernel doesn't leak any data. > > Signed-off-by: Joe Perches <joe@perches.com> This looks a lot better, applied. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] sctp: sctp_sendmsg: Don't test known non-null sinfo 2011-05-12 19:19 [PATCH 0/2] sctp: socket cleanups Joe Perches 2011-05-12 19:19 ` [PATCH 1/2] sctp: sctp_sendmsg: Don't initialize default_sinfo Joe Perches @ 2011-05-12 19:19 ` Joe Perches 2011-05-12 21:31 ` David Miller 1 sibling, 1 reply; 7+ messages in thread From: Joe Perches @ 2011-05-12 19:19 UTC (permalink / raw) To: Vlad Yasevich, Sridhar Samudrala Cc: David S. Miller, linux-sctp, netdev, linux-kernel It's already known non-null above. Signed-off-by: Joe Perches <joe@perches.com> --- net/sctp/socket.c | 10 ++++------ 1 files changed, 4 insertions(+), 6 deletions(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index a5d3560..10ebcd7 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -1793,12 +1793,10 @@ SCTP_STATIC int sctp_sendmsg(struct kiocb *iocb, struct sock *sk, goto out_free; } - if (sinfo) { - /* Check for invalid stream. */ - if (sinfo->sinfo_stream >= asoc->c.sinit_num_ostreams) { - err = -EINVAL; - goto out_free; - } + /* Check for invalid stream. */ + if (sinfo->sinfo_stream >= asoc->c.sinit_num_ostreams) { + err = -EINVAL; + goto out_free; } timeo = sock_sndtimeo(sk, msg->msg_flags & MSG_DONTWAIT); -- 1.7.5.rc3.dirty ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] sctp: sctp_sendmsg: Don't test known non-null sinfo 2011-05-12 19:19 ` [PATCH 2/2] sctp: sctp_sendmsg: Don't test known non-null sinfo Joe Perches @ 2011-05-12 21:31 ` David Miller 0 siblings, 0 replies; 7+ messages in thread From: David Miller @ 2011-05-12 21:31 UTC (permalink / raw) To: joe; +Cc: vladislav.yasevich, sri, linux-sctp, netdev, linux-kernel From: Joe Perches <joe@perches.com> Date: Thu, 12 May 2011 12:19:10 -0700 > It's already known non-null above. > > Signed-off-by: Joe Perches <joe@perches.com> Applied. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2011-05-12 21:31 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-05-12 19:19 [PATCH 0/2] sctp: socket cleanups Joe Perches 2011-05-12 19:19 ` [PATCH 1/2] sctp: sctp_sendmsg: Don't initialize default_sinfo Joe Perches 2011-05-12 21:06 ` David Miller 2011-05-12 21:27 ` [PATCH 1/2 v2] " Joe Perches 2011-05-12 21:31 ` David Miller 2011-05-12 19:19 ` [PATCH 2/2] sctp: sctp_sendmsg: Don't test known non-null sinfo Joe Perches 2011-05-12 21:31 ` David Miller
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).