From: Dan Carpenter <dan.carpenter@oracle.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Stephan Mueller <smueller@chronox.de>,
kbuild@01.org, linux-kernel@vger.kernel.org,
Linux Crypto Mailing List <linux-crypto@vger.kernel.org>
Subject: Re: [PATCH] Potential NULL pointer deference in drbg_ctr_df
Date: Fri, 4 Jul 2014 13:50:03 +0300 [thread overview]
Message-ID: <20140704105003.GA25934@mwanda> (raw)
In-Reply-To: <20140625090646.GB8727@gondor.apana.org.au>
On Wed, Jun 25, 2014 at 05:06:46PM +0800, Herbert Xu wrote:
> On Sat, Jun 21, 2014 at 02:26:29PM +0200, Stephan Mueller wrote:
> > The handling of additional input data / personalization string data may
> > be subject to a NULL pointer deference for the CTR DRBG. The
> > caller-provided data may be NULL which must be caught by the DRBG.
> >
> > Reported-by: kbuild test robot <fengguang.wu@intel.com>
Oh, huh. This bug was actually reported by me but I forgot to change
the from header and apparently my smtp server allows me to send emails
as if I work for Intel. :P
Fengguang is much nicer than I am. :P
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > ---
> > crypto/drbg.c | 23 +++++++++++++----------
> > 1 file changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/crypto/drbg.c b/crypto/drbg.c
> > index faaa2ce..8e7c302 100644
> > --- a/crypto/drbg.c
> > +++ b/crypto/drbg.c
> > @@ -513,17 +513,20 @@ static int drbg_ctr_df(struct drbg_state *drbg,
> > drbg_string_fill(&S2, L_N, sizeof(L_N));
> > drbg_string_fill(&S4, pad, padlen);
> > S1.next = &S2;
> > - S2.next = addtl;
> >
> > - /*
> > - * splice in addtl between S2 and S4 -- we place S4 at the end of the
> > - * input data chain
> > - */
> > - tempstr = addtl;
> > - for (; NULL != tempstr; tempstr = tempstr->next)
> > - if (NULL == tempstr->next)
> > - break;
> > - tempstr->next = &S4;
> > + if (NULL == addtl) {
> > + S2.next = &S4;
> > + } else {
> > + /*
> > + * splice in addtl between S2 and S4 -- we place S4 at the end
> > + * of the input data chain
> > + */
> > + S2.next = addtl;
> > + tempstr = addtl;
> > + while (tempstr->next)
> > + tempstr = tempstr->next;
> > + tempstr->next = &S4;
>
> You've still got exactly the same NULL dereference.
I was offline for a bit so I'm coming into this late. It's weird that
Stephan isn't defending his patch but it looks ok to me...
regards,
dan carpenter
WARNING: multiple messages have this Message-ID (diff)
From: Dan Carpenter <dan.carpenter@oracle.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Stephan Mueller <smueller@chronox.de>,
kbuild test robot <fengguang.wu@intel.com>,
kbuild@01.org, linux-kernel@vger.kernel.org,
Linux Crypto Mailing List <linux-crypto@vger.kernel.org>
Subject: Re: [PATCH] Potential NULL pointer deference in drbg_ctr_df
Date: Fri, 4 Jul 2014 13:50:03 +0300 [thread overview]
Message-ID: <20140704105003.GA25934@mwanda> (raw)
In-Reply-To: <20140625090646.GB8727@gondor.apana.org.au>
On Wed, Jun 25, 2014 at 05:06:46PM +0800, Herbert Xu wrote:
> On Sat, Jun 21, 2014 at 02:26:29PM +0200, Stephan Mueller wrote:
> > The handling of additional input data / personalization string data may
> > be subject to a NULL pointer deference for the CTR DRBG. The
> > caller-provided data may be NULL which must be caught by the DRBG.
> >
> > Reported-by: kbuild test robot <fengguang.wu@intel.com>
Oh, huh. This bug was actually reported by me but I forgot to change
the from header and apparently my smtp server allows me to send emails
as if I work for Intel. :P
Fengguang is much nicer than I am. :P
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > ---
> > crypto/drbg.c | 23 +++++++++++++----------
> > 1 file changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/crypto/drbg.c b/crypto/drbg.c
> > index faaa2ce..8e7c302 100644
> > --- a/crypto/drbg.c
> > +++ b/crypto/drbg.c
> > @@ -513,17 +513,20 @@ static int drbg_ctr_df(struct drbg_state *drbg,
> > drbg_string_fill(&S2, L_N, sizeof(L_N));
> > drbg_string_fill(&S4, pad, padlen);
> > S1.next = &S2;
> > - S2.next = addtl;
> >
> > - /*
> > - * splice in addtl between S2 and S4 -- we place S4 at the end of the
> > - * input data chain
> > - */
> > - tempstr = addtl;
> > - for (; NULL != tempstr; tempstr = tempstr->next)
> > - if (NULL == tempstr->next)
> > - break;
> > - tempstr->next = &S4;
> > + if (NULL == addtl) {
> > + S2.next = &S4;
> > + } else {
> > + /*
> > + * splice in addtl between S2 and S4 -- we place S4 at the end
> > + * of the input data chain
> > + */
> > + S2.next = addtl;
> > + tempstr = addtl;
> > + while (tempstr->next)
> > + tempstr = tempstr->next;
> > + tempstr->next = &S4;
>
> You've still got exactly the same NULL dereference.
I was offline for a bit so I'm coming into this late. It's weird that
Stephan isn't defending his patch but it looks ok to me...
regards,
dan carpenter
next prev parent reply other threads:[~2014-07-04 10:50 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20140620202418.GZ5015@mwanda>
2014-06-21 12:26 ` [PATCH] Potential NULL pointer deference in drbg_ctr_df Stephan Mueller
2014-06-25 9:06 ` Herbert Xu
2014-07-04 10:50 ` Dan Carpenter [this message]
2014-07-04 10:50 ` Dan Carpenter
2014-07-05 0:00 ` Stephan Mueller
2014-07-05 0:36 ` Fengguang Wu
2014-07-05 0:36 ` Fengguang Wu
[not found] ` <1455308.iM4tBB0QjZ@myon.chronox.de>
2014-06-25 9:07 ` [cryptodev:master 9/28] crypto/drbg.c:526 drbg_ctr_df() error: we previously assumed 'tempstr' could be null (see line 523) Herbert Xu
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=20140704105003.GA25934@mwanda \
--to=dan.carpenter@oracle.com \
--cc=herbert@gondor.apana.org.au \
--cc=kbuild@01.org \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=smueller@chronox.de \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.