Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <7621dbe1bfb4f461382952ebeaada5ce103eaf88.camel@chronox.de>
Subject: Re: A possible divide by zero bug in drbg_ctr_df
From:   Stephan Mueller <smueller@chronox.de>
To:     Herbert Xu <herbert@gondor.apana.org.au>,
        Yiyuan guo <yguoaz@gmail.com>
Cc:     linux-crypto@vger.kernel.org, davem@davemloft.net
Date:   Fri, 21 May 2021 09:27:03 +0200
In-Reply-To: <20210521064825.vhovv7sa5qif2f3j@gondor.apana.org.au>
References: <CAM7=BFrCTTuBkYb-ceX5C=e8VhAuWBVb_pYQ+K0LB1gn3h=hqA@mail.gmail.com>
         <20210521064825.vhovv7sa5qif2f3j@gondor.apana.org.au>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.38.4 (3.38.4-1.fc33) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Am Freitag, dem 21.05.2021 um 14:48 +0800 schrieb Herbert Xu:
> On Fri, May 21, 2021 at 11:23:36AM +0800, Yiyuan guo wrote:
> > In crypto/drbg.c, the function drbg_ctr_df has the following code:
> > 
> > padlen = (inputlen + sizeof(L_N) + 1) % (drbg_blocklen(drbg));
> > 
> > However, the function drbg_blocklen may return zero:
> > 
> > static inline __u8 drbg_blocklen(struct drbg_state *drbg)
> > {
> >     if (drbg && drbg->core)
> >         return drbg->core->blocklen_bytes;
> >     return 0;
> > }
> > 
> > Is it possible to trigger a divide by zero problem here?


I do not think there is a problem. Allow me to explain:

To reach the drbg_ctr_df function, the drbg_ctr_update function must be
called. This is either called from the seeding operation or the generate
operation.

The seeding operation is guarded as follows:

1. if called from the instantiation drbg_instantiate, we have:

        if (!drbg->core) {
                drbg->core = &drbg_cores[coreref];

2. if called from the generate function drbg_generate, we have:

	if (!drbg->core) {
                pr_devel("DRBG: not yet seeded\n");
                return -EINVAL;
        }

Thus, in both cases, when no drbg and no drbg->core is present, either the
code tries to get it or it fails before trying to invoke the concerning code
path.


When the drbg_ctr_update function is invoked from the generate operation, the
step 2 above applies.


Thus, when we reach the call for drbg_blocklen to get the padlen, we always
have a drbg and a drbg->core pointer.

In general, as soon as the DRBG code path reaches the DRBG-specific
implementations hiding behind drbg->[update|generate], the entire DRBG is
fully initialized and all pointers/memory is set up as needed.

The sanity check in drbg_blocklen is there as the function may be called in
earlier functions where it is not fully guaranteed that the drbg and drbg-
>core is set.

Thanks for the review.
Stephan