* Re: [PATCH v2] DRBG: simplify ordering of linked list in drbg_ctr_df [not found] ` <280704396.67z02n3FAf@myon.chronox.de> @ 2014-06-25 9:08 ` Herbert Xu 2014-06-26 6:45 ` Herbert Xu 0 siblings, 1 reply; 3+ messages in thread From: Herbert Xu @ 2014-06-25 9:08 UTC (permalink / raw) To: Stephan Mueller Cc: kbuild test robot, kbuild, linux-kernel, Dan Carpenter, Rafael Aquini, Linux Crypto Mailing List On Mon, Jun 23, 2014 at 09:11:29AM +0200, Stephan Mueller wrote: > As reported by a static code analyzer, the code for the ordering of > the linked list can be simplified. > > Reported-by: kbuild test robot <fengguang.wu@intel.com> > Signed-off-by: Stephan Mueller <smueller@chronox.de> > --- > crypto/drbg.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/crypto/drbg.c b/crypto/drbg.c > index faaa2ce..99fa8f8 100644 > --- a/crypto/drbg.c > +++ b/crypto/drbg.c > @@ -516,13 +516,13 @@ static int drbg_ctr_df(struct drbg_state *drbg, > S2.next = addtl; > > /* > - * splice in addtl between S2 and S4 -- we place S4 at the end of the > - * input data chain > + * Splice in addtl between S2 and S4 -- we place S4 at the end > + * of the input data chain. As this code is only triggered when > + * addtl is not NULL, no NULL checks are necessary. > */ > tempstr = addtl; > - for (; NULL != tempstr; tempstr = tempstr->next) > - if (NULL == tempstr->next) > - break; > + while (tempstr->next) > + tempstr = tempstr->next; > tempstr->next = &S4; This is still broken. -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] DRBG: simplify ordering of linked list in drbg_ctr_df 2014-06-25 9:08 ` [PATCH v2] DRBG: simplify ordering of linked list in drbg_ctr_df Herbert Xu @ 2014-06-26 6:45 ` Herbert Xu 2014-06-26 11:33 ` Stephan Mueller 0 siblings, 1 reply; 3+ messages in thread From: Herbert Xu @ 2014-06-26 6:45 UTC (permalink / raw) To: Stephan Mueller Cc: kbuild test robot, kbuild, linux-kernel, Dan Carpenter, Rafael Aquini, Linux Crypto Mailing List On Wed, Jun 25, 2014 at 05:08:28PM +0800, Herbert Xu wrote: > On Mon, Jun 23, 2014 at 09:11:29AM +0200, Stephan Mueller wrote: > > As reported by a static code analyzer, the code for the ordering of > > the linked list can be simplified. > > > > Reported-by: kbuild test robot <fengguang.wu@intel.com> > > Signed-off-by: Stephan Mueller <smueller@chronox.de> > > --- > > crypto/drbg.c | 10 +++++----- > > 1 file changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/crypto/drbg.c b/crypto/drbg.c > > index faaa2ce..99fa8f8 100644 > > --- a/crypto/drbg.c > > +++ b/crypto/drbg.c > > @@ -516,13 +516,13 @@ static int drbg_ctr_df(struct drbg_state *drbg, > > S2.next = addtl; > > > > /* > > - * splice in addtl between S2 and S4 -- we place S4 at the end of the > > - * input data chain > > + * Splice in addtl between S2 and S4 -- we place S4 at the end > > + * of the input data chain. As this code is only triggered when > > + * addtl is not NULL, no NULL checks are necessary. > > */ > > tempstr = addtl; > > - for (; NULL != tempstr; tempstr = tempstr->next) > > - if (NULL == tempstr->next) > > - break; > > + while (tempstr->next) > > + tempstr = tempstr->next; > > tempstr->next = &S4; > > This is still broken. OK I take that back. As addtl is not NULL neither version will do a NULL derference. But I will apply your cleanup patch anyway. Thanks, -- Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] DRBG: simplify ordering of linked list in drbg_ctr_df 2014-06-26 6:45 ` Herbert Xu @ 2014-06-26 11:33 ` Stephan Mueller 0 siblings, 0 replies; 3+ messages in thread From: Stephan Mueller @ 2014-06-26 11:33 UTC (permalink / raw) To: Herbert Xu Cc: kbuild test robot, kbuild, linux-kernel, Dan Carpenter, Rafael Aquini, Linux Crypto Mailing List Am Donnerstag, 26. Juni 2014, 14:45:42 schrieb Herbert Xu: Hi Herbert, >On Wed, Jun 25, 2014 at 05:08:28PM +0800, Herbert Xu wrote: >> On Mon, Jun 23, 2014 at 09:11:29AM +0200, Stephan Mueller wrote: >> > As reported by a static code analyzer, the code for the ordering of >> > the linked list can be simplified. >> > >> > Reported-by: kbuild test robot <fengguang.wu@intel.com> >> > Signed-off-by: Stephan Mueller <smueller@chronox.de> >> > --- >> > >> > crypto/drbg.c | 10 +++++----- >> > 1 file changed, 5 insertions(+), 5 deletions(-) >> > >> > diff --git a/crypto/drbg.c b/crypto/drbg.c >> > index faaa2ce..99fa8f8 100644 >> > --- a/crypto/drbg.c >> > +++ b/crypto/drbg.c >> > @@ -516,13 +516,13 @@ static int drbg_ctr_df(struct drbg_state >> > *drbg, >> > >> > S2.next = addtl; >> > >> > /* >> > >> > - * splice in addtl between S2 and S4 -- we place S4 at the end of >> > the - * input data chain >> > + * Splice in addtl between S2 and S4 -- we place S4 at the end >> > + * of the input data chain. As this code is only triggered when >> > + * addtl is not NULL, no NULL checks are necessary. >> > >> > */ >> > >> > tempstr = addtl; >> > >> > - for (; NULL != tempstr; tempstr = tempstr->next) >> > - if (NULL == tempstr->next) >> > - break; >> > + while (tempstr->next) >> > + tempstr = tempstr->next; >> > >> > tempstr->next = &S4; >> >> This is still broken. > >OK I take that back. As addtl is not NULL neither version will >do a NULL derference. But I will apply your cleanup patch anyway. When I wrote my first patch considering the NULL pointer, I was already wondering why during my tests I did not observe any crasher. In case the NULL pointer dereference would have been real, it would need to have crashed when pulling random bytes via the kernel crypto API -- I have a test that iterates over all DRBG types, instantiates them and pulls up to 100,000 bytes. If the NULL pointer dereference would have been real, the following call sequences triggered by normal kernel crypto API usage should have triggered it, because they all set addtl to NULL. crypto_rng_get_bytes --> drbg_kcapi_random with slen >0 --> drbg_generate_long(drbg, rdata, dlen, NULL); --> drbg_generate(drbg, rdata, dlen, NULL); --> drbg_ctr_generate(..., NULL) And here, the following is only called when addtl is not NULL --> drbg_ctr_update --> drbg_ctr_df Ciao Stephan ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-06-26 11:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <5069712.neqH4zuI9U@myon.chronox.de>
[not found] ` <280704396.67z02n3FAf@myon.chronox.de>
2014-06-25 9:08 ` [PATCH v2] DRBG: simplify ordering of linked list in drbg_ctr_df Herbert Xu
2014-06-26 6:45 ` Herbert Xu
2014-06-26 11:33 ` Stephan Mueller
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).