* [PATCH net] sctp: prevent info leak in sctp_make_heartbeat()
@ 2021-06-29 8:19 Dan Carpenter
2021-06-29 8:23 ` Andreas Fink
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2021-06-29 8:19 UTC (permalink / raw)
To: Vlad Yasevich, Xin Long
Cc: Neil Horman, Marcelo Ricardo Leitner, David S. Miller,
Jakub Kicinski, linux-sctp, netdev, kernel-janitors
The "hbinfo" struct has a 4 byte hole at the end so we have to zero it
out to prevent stack information from being disclosed.
Fixes: fe59379b9ab7 ("sctp: do the basic send and recv for PLPMTUD probe")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Btw = {} is the newest way to initialize holes.
In the past we have debated whether = {} will *always* zero out struct
holes and it wasn't clear from the C standard. But it turns out that
"= {}" is not part of the standard but is instead a GCC extension and it
does clear the holes. In GCC (not the C standard) then = {0}; is also
supposed to initialize holes in there was a bug in one version where it
didn't.
So that's nice, because adding memset()s to zero everywhere was ugly.
net/sctp/sm_make_chunk.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 587fb3cb88e2..3a290f620e96 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -1162,7 +1162,7 @@ struct sctp_chunk *sctp_make_new_encap_port(const struct sctp_association *asoc,
struct sctp_chunk *sctp_make_heartbeat(const struct sctp_association *asoc,
const struct sctp_transport *transport)
{
- struct sctp_sender_hb_info hbinfo;
+ struct sctp_sender_hb_info hbinfo = {};
struct sctp_chunk *retval;
retval = sctp_make_control(asoc, SCTP_CID_HEARTBEAT, 0,
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH net] sctp: prevent info leak in sctp_make_heartbeat()
2021-06-29 8:19 [PATCH net] sctp: prevent info leak in sctp_make_heartbeat() Dan Carpenter
@ 2021-06-29 8:23 ` Andreas Fink
2021-06-29 9:13 ` Dan Carpenter
2021-06-29 10:37 ` David Laight
0 siblings, 2 replies; 4+ messages in thread
From: Andreas Fink @ 2021-06-29 8:23 UTC (permalink / raw)
To: Dan Carpenter
Cc: Vlad Yasevich, Xin Long, Neil Horman, Marcelo Ricardo Leitner,
David S. Miller, Jakub Kicinski, linux-sctp, netdev,
kernel-janitors
Does that gcc extension work with all compilers, especially clang?
Dan Carpenter wrote on 29.06.21 10:19:
> The "hbinfo" struct has a 4 byte hole at the end so we have to zero it
> out to prevent stack information from being disclosed.
>
> Fixes: fe59379b9ab7 ("sctp: do the basic send and recv for PLPMTUD probe")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Btw = {} is the newest way to initialize holes.
>
> In the past we have debated whether = {} will *always* zero out struct
> holes and it wasn't clear from the C standard. But it turns out that
> "= {}" is not part of the standard but is instead a GCC extension and it
> does clear the holes. In GCC (not the C standard) then = {0}; is also
> supposed to initialize holes in there was a bug in one version where it
> didn't.
>
> So that's nice, because adding memset()s to zero everywhere was ugly.
>
> net/sctp/sm_make_chunk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
> index 587fb3cb88e2..3a290f620e96 100644
> --- a/net/sctp/sm_make_chunk.c
> +++ b/net/sctp/sm_make_chunk.c
> @@ -1162,7 +1162,7 @@ struct sctp_chunk *sctp_make_new_encap_port(const struct sctp_association *asoc,
> struct sctp_chunk *sctp_make_heartbeat(const struct sctp_association *asoc,
> const struct sctp_transport *transport)
> {
> - struct sctp_sender_hb_info hbinfo;
> + struct sctp_sender_hb_info hbinfo = {};
> struct sctp_chunk *retval;
>
> retval = sctp_make_control(asoc, SCTP_CID_HEARTBEAT, 0,
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH net] sctp: prevent info leak in sctp_make_heartbeat()
2021-06-29 8:23 ` Andreas Fink
@ 2021-06-29 9:13 ` Dan Carpenter
2021-06-29 10:37 ` David Laight
1 sibling, 0 replies; 4+ messages in thread
From: Dan Carpenter @ 2021-06-29 9:13 UTC (permalink / raw)
To: Andreas Fink
Cc: Vlad Yasevich, Xin Long, Neil Horman, Marcelo Ricardo Leitner,
David S. Miller, Jakub Kicinski, linux-sctp, netdev,
kernel-janitors
On Tue, Jun 29, 2021 at 10:23:49AM +0200, Andreas Fink wrote:
> Does that gcc extension work with all compilers, especially clang?
>
I had to look up the thread where this was discussed.
https://lore.kernel.org/lkml/20200731045301.GI75549@unreal/
I was wrong. It started as a GCC extension but was made part of the
standard in C11. So we should be good.
https://lore.kernel.org/lkml/20200731140452.GE24045@ziepe.ca/
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH net] sctp: prevent info leak in sctp_make_heartbeat()
2021-06-29 8:23 ` Andreas Fink
2021-06-29 9:13 ` Dan Carpenter
@ 2021-06-29 10:37 ` David Laight
1 sibling, 0 replies; 4+ messages in thread
From: David Laight @ 2021-06-29 10:37 UTC (permalink / raw)
To: 'Andreas Fink', Dan Carpenter
Cc: Vlad Yasevich, Xin Long, Neil Horman, Marcelo Ricardo Leitner,
David S. Miller, Jakub Kicinski, linux-sctp, netdev,
kernel-janitors
> > - struct sctp_sender_hb_info hbinfo;
> > + struct sctp_sender_hb_info hbinfo = {};
> Does that gcc extension work with all compilers, especially clang?
> > So that's nice, because adding memset()s to zero everywhere was ugly.
The 'real fun' (tm) starts when the bit pattern for the NULL
pointer isn't 'all zeros'.
Using memset() is then broken - I suspect the compiler is
expected to initialise pointers to the correct NULL pattern.
Not that I think anyone sane would consider trying to compile
any 'normal' C code for such a system.
OTOH it is probably why clang is bleating about (int)((char *)0 + 4)
being undefined behaviour.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-06-29 10:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-29 8:19 [PATCH net] sctp: prevent info leak in sctp_make_heartbeat() Dan Carpenter
2021-06-29 8:23 ` Andreas Fink
2021-06-29 9:13 ` Dan Carpenter
2021-06-29 10:37 ` David Laight
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).