All of lore.kernel.org
 help / color / mirror / Atom feed
From: Corentin Labbe <clabbe.montjoie@gmail.com>
To: Artem Savkov <asavkov@redhat.com>
Cc: Artem Savkov <artem.savkov@gmail.com>,
	herbert@gondor.apana.org.au, linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] sun4i_ss_prng: fix return value of sun4i_ss_prng_generate
Date: Wed, 7 Feb 2018 14:12:14 +0100	[thread overview]
Message-ID: <20180207131214.GC19249@Red> (raw)
In-Reply-To: <20180207105201.pnn2s57wdk5zuda2@shodan.usersys.redhat.com>

On Wed, Feb 07, 2018 at 11:52:01AM +0100, Artem Savkov wrote:
> On Wed, Feb 07, 2018 at 10:56:59AM +0100, Corentin Labbe wrote:
> > On Tue, Feb 06, 2018 at 10:20:21PM +0100, Artem Savkov wrote:
> > > According to crypto/rng.h generate function should return 0 on success
> > > and < 0 on error.
> > > 
> > > Fixes: b8ae5c7387ad ("crypto: sun4i-ss - support the Security System PRNG")
> > > Signed-off-by: Artem Savkov <artem.savkov@gmail.com>
> > > ---
> > >  drivers/crypto/sunxi-ss/sun4i-ss-prng.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-prng.c b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
> > > index 0d01d1624252..5754e0b92fb0 100644
> > > --- a/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
> > > +++ b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
> > > @@ -52,5 +52,5 @@ int sun4i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src,
> > >  
> > >  	writel(0, ss->base + SS_CTL);
> > >  	spin_unlock(&ss->slock);
> > > -	return dlen;
> > > +	return 0;
> > >  }
> > > -- 
> > > 2.15.1
> > > 
> > 
> > According to Documentation/crypto/api-samples.rst ("Code Example For Random Number Generator Usage")
> > you must return the length of data generated.
> 
> I don't think that example is the same as rng_alg.generate, it has a
> different protottype.

crypto_rng_get_bytes() use crypto_rng_generate() which work on crypto_rng

> 
> > So crypto_rng_generate/crypto_rng_get_bytes documentation in crypto/rng.h must be fixed.
> 
> It's not just documentation, every other rng driver returns it this way
> and it gets aead_init_geniv() (and subsequently crypto_create_tfm())
> really confused because they expect the return value to be 0 or < 0.
> 

Perhaps the example is wrong because "copied from/inspirated by" hwrng API which have this behavior.

After checking, other crypto_rng driver, you are probably right, so you can add:
Acked-by: Corentin Labbe <clabbe.montjoie@gmail.com>

Thanks
Regards

WARNING: multiple messages have this Message-ID (diff)
From: clabbe.montjoie@gmail.com (Corentin Labbe)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] sun4i_ss_prng: fix return value of sun4i_ss_prng_generate
Date: Wed, 7 Feb 2018 14:12:14 +0100	[thread overview]
Message-ID: <20180207131214.GC19249@Red> (raw)
In-Reply-To: <20180207105201.pnn2s57wdk5zuda2@shodan.usersys.redhat.com>

On Wed, Feb 07, 2018 at 11:52:01AM +0100, Artem Savkov wrote:
> On Wed, Feb 07, 2018 at 10:56:59AM +0100, Corentin Labbe wrote:
> > On Tue, Feb 06, 2018 at 10:20:21PM +0100, Artem Savkov wrote:
> > > According to crypto/rng.h generate function should return 0 on success
> > > and < 0 on error.
> > > 
> > > Fixes: b8ae5c7387ad ("crypto: sun4i-ss - support the Security System PRNG")
> > > Signed-off-by: Artem Savkov <artem.savkov@gmail.com>
> > > ---
> > >  drivers/crypto/sunxi-ss/sun4i-ss-prng.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/crypto/sunxi-ss/sun4i-ss-prng.c b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
> > > index 0d01d1624252..5754e0b92fb0 100644
> > > --- a/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
> > > +++ b/drivers/crypto/sunxi-ss/sun4i-ss-prng.c
> > > @@ -52,5 +52,5 @@ int sun4i_ss_prng_generate(struct crypto_rng *tfm, const u8 *src,
> > >  
> > >  	writel(0, ss->base + SS_CTL);
> > >  	spin_unlock(&ss->slock);
> > > -	return dlen;
> > > +	return 0;
> > >  }
> > > -- 
> > > 2.15.1
> > > 
> > 
> > According to Documentation/crypto/api-samples.rst ("Code Example For Random Number Generator Usage")
> > you must return the length of data generated.
> 
> I don't think that example is the same as rng_alg.generate, it has a
> different protottype.

crypto_rng_get_bytes() use crypto_rng_generate() which work on crypto_rng

> 
> > So crypto_rng_generate/crypto_rng_get_bytes documentation in crypto/rng.h must be fixed.
> 
> It's not just documentation, every other rng driver returns it this way
> and it gets aead_init_geniv() (and subsequently crypto_create_tfm())
> really confused because they expect the return value to be 0 or < 0.
> 

Perhaps the example is wrong because "copied from/inspirated by" hwrng API which have this behavior.

After checking, other crypto_rng driver, you are probably right, so you can add:
Acked-by: Corentin Labbe <clabbe.montjoie@gmail.com>

Thanks
Regards

  reply	other threads:[~2018-02-07 13:12 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-06 21:20 [PATCH 0/2] sun4i_ss_prng fixes Artem Savkov
2018-02-06 21:20 ` Artem Savkov
2018-02-06 21:20 ` [PATCH 1/2] sun4i_ss_prng: fix return value of sun4i_ss_prng_generate Artem Savkov
2018-02-06 21:20   ` Artem Savkov
2018-02-06 21:20   ` Artem Savkov
2018-02-07  9:56   ` Corentin Labbe
2018-02-07  9:56     ` Corentin Labbe
2018-02-07 10:52     ` Artem Savkov
2018-02-07 10:52       ` Artem Savkov
2018-02-07 13:12       ` Corentin Labbe [this message]
2018-02-07 13:12         ` Corentin Labbe
2018-02-06 21:20 ` [PATCH 2/2] sun4i_ss_prng: convert lock to _bh in sun4i_ss_prng_generate Artem Savkov
2018-02-06 21:20   ` Artem Savkov
2018-02-09 15:51 ` [PATCH 0/2] sun4i_ss_prng fixes Herbert Xu
2018-02-09 15:51   ` 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=20180207131214.GC19249@Red \
    --to=clabbe.montjoie@gmail.com \
    --cc=artem.savkov@gmail.com \
    --cc=asavkov@redhat.com \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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.