* sctp use-uninitialized warning in net-2.6.25 @ 2008-01-16 21:59 Andrew Morton 2008-01-18 12:49 ` David Miller 0 siblings, 1 reply; 8+ messages in thread From: Andrew Morton @ 2008-01-16 21:59 UTC (permalink / raw) To: netdev net/sctp/sm_statefuns.c: In function 'sctp_sf_do_5_1C_ack': net/sctp/sm_statefuns.c:484: warning: 'error' may be used uninitialized in this function It is not obvious that this is a false positive. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: sctp use-uninitialized warning in net-2.6.25 2008-01-16 21:59 sctp use-uninitialized warning in net-2.6.25 Andrew Morton @ 2008-01-18 12:49 ` David Miller 2008-01-18 16:03 ` Vlad Yasevich 0 siblings, 1 reply; 8+ messages in thread From: David Miller @ 2008-01-18 12:49 UTC (permalink / raw) To: akpm; +Cc: netdev, vladislav.yasevich From: Andrew Morton <akpm@linux-foundation.org> Date: Wed, 16 Jan 2008 13:59:57 -0800 > net/sctp/sm_statefuns.c: In function 'sctp_sf_do_5_1C_ack': > net/sctp/sm_statefuns.c:484: warning: 'error' may be used uninitialized in this function > > It is not obvious that this is a false positive. I'll check in the following for now. Vlad, please take a look. diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index b126751..6e12757 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -481,7 +481,7 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct sctp_endpoint *ep, sctp_init_chunk_t *initchunk; struct sctp_chunk *err_chunk; struct sctp_packet *packet; - sctp_error_t error; + sctp_error_t error = SCTP_ERROR_NO_ERROR; if (!sctp_vtag_verify(chunk, asoc)) return sctp_sf_pdiscard(ep, asoc, type, arg, commands); ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: sctp use-uninitialized warning in net-2.6.25 2008-01-18 12:49 ` David Miller @ 2008-01-18 16:03 ` Vlad Yasevich 2008-01-18 23:37 ` David Miller 0 siblings, 1 reply; 8+ messages in thread From: Vlad Yasevich @ 2008-01-18 16:03 UTC (permalink / raw) To: David Miller; +Cc: akpm, netdev David Miller wrote: > From: Andrew Morton <akpm@linux-foundation.org> > Date: Wed, 16 Jan 2008 13:59:57 -0800 > >> net/sctp/sm_statefuns.c: In function 'sctp_sf_do_5_1C_ack': >> net/sctp/sm_statefuns.c:484: warning: 'error' may be used uninitialized in this function >> >> It is not obvious that this is a false positive. > > I'll check in the following for now. > > Vlad, please take a look. > > diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c > index b126751..6e12757 100644 > --- a/net/sctp/sm_statefuns.c > +++ b/net/sctp/sm_statefuns.c > @@ -481,7 +481,7 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct sctp_endpoint *ep, > sctp_init_chunk_t *initchunk; > struct sctp_chunk *err_chunk; > struct sctp_packet *packet; > - sctp_error_t error; > + sctp_error_t error = SCTP_ERROR_NO_ERROR; > > if (!sctp_vtag_verify(chunk, asoc)) > return sctp_sf_pdiscard(ep, asoc, type, arg, commands); > Hi David We can do that, or move the declaration to the only block that uses it. Like this: diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index f02ce3d..193c0c2 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -442,7 +442,6 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct sctp_endpoint *ep, sctp_init_chunk_t *initchunk; struct sctp_chunk *err_chunk; struct sctp_packet *packet; - sctp_error_t error; if (!sctp_vtag_verify(chunk, asoc)) return sctp_sf_pdiscard(ep, asoc, type, arg, commands); @@ -466,6 +465,7 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct sctp_endpoint *ep, if (!sctp_verify_init(asoc, chunk->chunk_hdr->type, (sctp_init_chunk_t *)chunk->chunk_hdr, chunk, &err_chunk)) { + sctp_error_t error; SCTP_INC_STATS(SCTP_MIB_ABORTEDS); -vlad ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: sctp use-uninitialized warning in net-2.6.25 2008-01-18 16:03 ` Vlad Yasevich @ 2008-01-18 23:37 ` David Miller 2008-01-19 2:17 ` Vlad Yasevich 0 siblings, 1 reply; 8+ messages in thread From: David Miller @ 2008-01-18 23:37 UTC (permalink / raw) To: vladislav.yasevich; +Cc: akpm, netdev From: Vlad Yasevich <vladislav.yasevich@hp.com> Date: Fri, 18 Jan 2008 11:03:20 -0500 > We can do that, or move the declaration to the only block that uses it. > Like this: ... > @@ -466,6 +465,7 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct sctp_endpoint *ep, > if (!sctp_verify_init(asoc, chunk->chunk_hdr->type, > (sctp_init_chunk_t *)chunk->chunk_hdr, chunk, > &err_chunk)) { > + sctp_error_t error; > > SCTP_INC_STATS(SCTP_MIB_ABORTEDS); > It's still potentially used uninitialized. It will only get set if err_chunk is non-zero. But even if err_chunk is zero, we try to use this variable. That's the whole problem, simply moving the variable to a different scope is not going to fix anything. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: sctp use-uninitialized warning in net-2.6.25 2008-01-18 23:37 ` David Miller @ 2008-01-19 2:17 ` Vlad Yasevich 2008-01-19 4:37 ` David Miller 0 siblings, 1 reply; 8+ messages in thread From: Vlad Yasevich @ 2008-01-19 2:17 UTC (permalink / raw) To: David Miller; +Cc: akpm, netdev David Miller wrote: > From: Vlad Yasevich <vladislav.yasevich@hp.com> > Date: Fri, 18 Jan 2008 11:03:20 -0500 > >> We can do that, or move the declaration to the only block that uses it. >> Like this: > ... >> @@ -466,6 +465,7 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct sctp_endpoint *ep, >> if (!sctp_verify_init(asoc, chunk->chunk_hdr->type, >> (sctp_init_chunk_t *)chunk->chunk_hdr, chunk, >> &err_chunk)) { >> + sctp_error_t error; >> >> SCTP_INC_STATS(SCTP_MIB_ABORTEDS); >> > > It's still potentially used uninitialized. > > It will only get set if err_chunk is non-zero. > > But even if err_chunk is zero, we try to use this > variable. > > That's the whole problem, simply moving the variable to > a different scope is not going to fix anything. > Hmm... in the code I am looking at, it's set in both zero and non-zero cases so it does solve the issue. So does initializing it to NO_ERROR like you did. -vlad ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: sctp use-uninitialized warning in net-2.6.25 2008-01-19 2:17 ` Vlad Yasevich @ 2008-01-19 4:37 ` David Miller 2008-01-20 10:47 ` Vlad Yasevich 0 siblings, 1 reply; 8+ messages in thread From: David Miller @ 2008-01-19 4:37 UTC (permalink / raw) To: vladislav.yasevich; +Cc: akpm, netdev From: Vlad Yasevich <vladislav.yasevich@hp.com> Date: Fri, 18 Jan 2008 21:17:56 -0500 > Hmm... in the code I am looking at, it's set in both zero and > non-zero cases so it does solve the issue. > > So does initializing it to NO_ERROR like you did. Here is the code block in question in net-2.6.25: /* Verify the INIT chunk before processing it. */ err_chunk = NULL; if (!sctp_verify_init(asoc, chunk->chunk_hdr->type, (sctp_init_chunk_t *)chunk->chunk_hdr, chunk, &err_chunk)) { ... if (err_chunk) { ... if (packet) { sctp_add_cmd_sf(commands, SCTP_CMD_SEND_PKT, SCTP_PACKET(packet)); SCTP_INC_STATS(SCTP_MIB_OUTCTRLCHUNKS); error = SCTP_ERROR_INV_PARAM; } else { error = SCTP_ERROR_NO_RESOURCE; } } ... return sctp_stop_t1_and_abort(commands, error, ECONNREFUSED, asoc, chunk->transport); If err_chunk == NULL at the "if (err_chunk)" test, error will be left uninitialized, even after being moved as you have suggested (right after the sctp_verify_init() call). Thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: sctp use-uninitialized warning in net-2.6.25 2008-01-19 4:37 ` David Miller @ 2008-01-20 10:47 ` Vlad Yasevich 2008-01-20 14:11 ` David Miller 0 siblings, 1 reply; 8+ messages in thread From: Vlad Yasevich @ 2008-01-20 10:47 UTC (permalink / raw) To: David Miller; +Cc: akpm, netdev [-- Attachment #1: Type: text/plain, Size: 1296 bytes --] David Miller wrote: > From: Vlad Yasevich <vladislav.yasevich@hp.com> > Date: Fri, 18 Jan 2008 21:17:56 -0500 > >> Hmm... in the code I am looking at, it's set in both zero and >> non-zero cases so it does solve the issue. >> >> So does initializing it to NO_ERROR like you did. > > Here is the code block in question in net-2.6.25: > > /* Verify the INIT chunk before processing it. */ > err_chunk = NULL; > if (!sctp_verify_init(asoc, chunk->chunk_hdr->type, > (sctp_init_chunk_t *)chunk->chunk_hdr, chunk, > &err_chunk)) { > ... > if (err_chunk) { > ... > if (packet) { > sctp_add_cmd_sf(commands, SCTP_CMD_SEND_PKT, > SCTP_PACKET(packet)); > SCTP_INC_STATS(SCTP_MIB_OUTCTRLCHUNKS); > error = SCTP_ERROR_INV_PARAM; > } else { > error = SCTP_ERROR_NO_RESOURCE; > } > } > ... > return sctp_stop_t1_and_abort(commands, error, ECONNREFUSED, > asoc, chunk->transport); > > If err_chunk == NULL at the "if (err_chunk)" test, error > will be left uninitialized, even after being moved as you > have suggested (right after the sctp_verify_init() call). > > Thanks. > Hi David Thanks for beating into my thick scull that this is in 2.6.25. I missed that initially. Anyway, here is a patch that sets the correct value. -vlad [-- Attachment #2: 0001-SCTP-Correctly-initialize-error-when-parameter-val.patch --] [-- Type: text/x-patch, Size: 1821 bytes --] >From 4788563632fae22023fc0d75b525d2d5f8e0735b Mon Sep 17 00:00:00 2001 From: Vlad Yasevich <vladislav.yasevich@hp.com> Date: Sun, 20 Jan 2008 00:22:06 -0500 Subject: [PATCH] [SCTP] Correctly initialize error when parameter validation failed. When parameter validation fails, there should be error causes that specify what type of failure we've encountered. If the causes are not there, we lacked memory to allocated them. Thus make that the default value for the error. Signed-off-by: Vlad Yasevich <vladislav.yasevich@hp.com> --- net/sctp/sm_statefuns.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c index 6e12757..da5497e 100644 --- a/net/sctp/sm_statefuns.c +++ b/net/sctp/sm_statefuns.c @@ -481,7 +481,6 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct sctp_endpoint *ep, sctp_init_chunk_t *initchunk; struct sctp_chunk *err_chunk; struct sctp_packet *packet; - sctp_error_t error = SCTP_ERROR_NO_ERROR; if (!sctp_vtag_verify(chunk, asoc)) return sctp_sf_pdiscard(ep, asoc, type, arg, commands); @@ -506,6 +505,8 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct sctp_endpoint *ep, (sctp_init_chunk_t *)chunk->chunk_hdr, chunk, &err_chunk)) { + sctp_error_t error = SCTP_ERROR_NO_RESOURCE; + /* This chunk contains fatal error. It is to be discarded. * Send an ABORT, with causes. If there are no causes, * then there wasn't enough memory. Just terminate @@ -525,9 +526,7 @@ sctp_disposition_t sctp_sf_do_5_1C_ack(const struct sctp_endpoint *ep, SCTP_PACKET(packet)); SCTP_INC_STATS(SCTP_MIB_OUTCTRLCHUNKS); error = SCTP_ERROR_INV_PARAM; - } else { - error = SCTP_ERROR_NO_RESOURCE; - } + } } /* SCTP-AUTH, Section 6.3: -- 1.5.2.5 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: sctp use-uninitialized warning in net-2.6.25 2008-01-20 10:47 ` Vlad Yasevich @ 2008-01-20 14:11 ` David Miller 0 siblings, 0 replies; 8+ messages in thread From: David Miller @ 2008-01-20 14:11 UTC (permalink / raw) To: vyasevich; +Cc: akpm, netdev From: Vlad Yasevich <vyasevich@verizon.net> Date: Sun, 20 Jan 2008 05:47:13 -0500 > Anyway, here is a patch that sets the correct value. Applied, but I had to fix up the trailing whitespace on line 29 of your patch. :-/ Thanks. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2008-01-20 14:11 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-01-16 21:59 sctp use-uninitialized warning in net-2.6.25 Andrew Morton 2008-01-18 12:49 ` David Miller 2008-01-18 16:03 ` Vlad Yasevich 2008-01-18 23:37 ` David Miller 2008-01-19 2:17 ` Vlad Yasevich 2008-01-19 4:37 ` David Miller 2008-01-20 10:47 ` Vlad Yasevich 2008-01-20 14:11 ` 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).