oles)
crypto/crypto_user_stat.c:295 crypto_reportstat_one() warn: check that 'rl'
doesn't leak information (struct has holes)
regards,
dan carpenter
046 tx_skb_finalize(skb);
1047 push_frames_if_head(sk);
1048 }
regards,
dan carpenter
^^
Otherwise the error handling needs a check as well.
1263 aalg = >pdata->aead_algs_info->algs_list[i];
1264 crypto_unregister_aead(aalg);
regards,
dan carpenter
862 sec_unmap_sg_on_err(skreq->src, steps, splits_in,
splits_in_nents,
863 sec_req->len_in, info->dev);
864 err_free_split_sizes:
865 kfree(split_sizes);
^^^^^^^
Double free.
866
867 return ret;
868 }
regards,
dan carpenter
}
401
402 psp_master->api_major = status->api_major;
403 psp_master->api_minor = status->api_minor;
404 psp_master->build = status->build;
405
406 return 0;
407 }
regards,
dan carpenter
code instead then we free skb which is pretty
subtle so far as APIs are concerned.
Looking at it now, I think we probably should be freeing skb on those
paths. The current code looks leaky to me.
351
352 return 0;
353 }
regards,
dan carpenter
current_timeo = *timeo_p;
> + if (current_timeo != MAX_SCHEDULE_TIMEOUT) {
> + current_timeo -= vm_wait;
> + if (current_timeo < 0)
> + current_timeo = 0;
> +
Hi Wenwen,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on cryptodev/master]
[also build test WARNING on v4.17-rc3 next-20180504]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
On Thu, Apr 26, 2018 at 09:52:57AM +0300, Dan Carpenter wrote:
> On Wed, Apr 25, 2018 at 08:36:22PM +0530, Atul Gupta wrote:
> > Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
> > Signed-off-by: Atul Gupta <atul.gu...@chelsio.com>
> > ---
> > driver
On Wed, Apr 25, 2018 at 08:36:22PM +0530, Atul Gupta wrote:
> Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
> Signed-off-by: Atul Gupta <atul.gu...@chelsio.com>
> ---
> drivers/crypto/chelsio/chtls/chtls_main.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 d
On Wed, Apr 25, 2018 at 08:35:32PM +0530, Atul Gupta wrote:
> - unindented continue
> - check for null page
> - signed return
>
> Reported-by: Dan Carpenter <dan.carpen...@oracle.com>
> Signed-off-by: Atul Gupta <atul.gu...@chelsio.com>
> ---
> drivers
l (16 vs 32)
drivers/crypto/chelsio/chtls/chtls_hw.c:277 chtls_key_info() error:
'__memset()' 'gcm_ctx->key' too small (16 vs 32)
regards,
dan carpenter
ULPCB_FLAG_NO_APPEND) ||
Check
1157 copy <= 0) {
1158 new_buf:
regards,
dan carpenter
0;
450 }
regards,
dan carpenter
release_sock(sk);
1426 lock_sock(sk);
1427 continue;
1428 }
1429 break;
1430 }
regards,
dan carpenter
return -EFAULT;
This has a signedness bug and the caller doesn't check for errors.
914 return (__force u16)cpu_to_be16(thdr->length);
915 }
regards,
dan carpenter
goto out;
1223
1224 out_err:
1225 if (csk_conn_inline(csk))
regards,
dan carpenter
1261 err_aead_algs:
1262 for (i = dd->pdata->aead_algs_info->registered - 1; i >= 0;
i--) {
^
Unchecked dereference.
1263 aalg = >pdata->aead_algs_info->algs_list[i];
1264 crypto_unregister_aead(aalg);
regards,
dan carpenter
We removed some if statements but left these statements indented too
far.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/crypto/chelsio/chcr_algo.c
b/drivers/crypto/chelsio/chcr_algo.c
index a9c894bf9c01..34a02d690548 100644
--- a/drivers/crypto/chelsio/chcr_
th kzfree()'
security/apparmor/crypto.c:102 aa_calc_profile_hash() warn: should '(struct
aa_profile)->hash' be freed with kzfree()'
regards,
dan carpenter
error %#x\n",
error);
^
Generally not set on the error paths.
780 goto err;
781 }
782
783 /* Display SEV firmware version */
regards,
dan carpenter
These lines are less than 80 characters so we don't need to break them
up into chunks.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/staging/ccree/cc_aead.c b/drivers/staging/ccree/cc_aead.c
index 265adffdab41..b58413172231 100644
--- a/drivers/staging
_sync_single_for_device(dev, ctx_p->user.key_dma_addr,
370 max_key_buf_size, DMA_TO_DEVICE);
371 ctx_p->keylen = keylen;
372
373 dev_dbg(dev, "return safely");
^
One extra space.
374 return 0;
375 }
regards,
dan carpenter
"val" needs to be signed for the error handling to work.
Fixes: 6cd225cc5d8a ("hwrng: exynos - add Samsung Exynos True RNG driver")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/char/hw_random/exynos-trng.c
b/drivers/char/hw_random/exynos
On Thu, Dec 07, 2017 at 09:00:11AM +0200, Gilad Ben-Yossef wrote:
> On Mon, Dec 4, 2017 at 11:36 AM, Dan Carpenter <dan.carpen...@oracle.com>
> wrote:
> > On Sun, Dec 03, 2017 at 01:58:12PM +, Gilad Ben-Yossef wrote:
> >> The ccree drivers was marking a lot
8 *)q->q.desc + (key_len - left);
But I can't test this.
432 }
433 }
434 /* Copy CPL TX PKT XT */
435 pos = copy_cpltx_pktxt(skb, dev, pos);
regards,
dan carpenter
inline hints... It probably would make
single line functions inline anyway.
regards,
dan carpenter
Looks good. Thanks!
regards,
dan carpenter
On Tue, Nov 14, 2017 at 11:33:20AM +0200, Gilad Ben-Yossef wrote:
> On Mon, Nov 13, 2017 at 8:33 PM, Dan Carpenter <dan.carpen...@oracle.com>
> wrote:
> > These cleanups look nice. Thanks.
> >
> > I hope you do a mass remove of likely/unlikely in a patch soon.
&g
These cleanups look nice. Thanks.
I hope you do a mass remove of likely/unlikely in a patch soon.
Whenever, I see one of those in a + line I always have to remind myself
that you're planning to do it in a later patch.
regards,
dan carpenter
This code seems correct, but the goto was indented too far.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/crypto/s5p-sss.c b/drivers/crypto/s5p-sss.c
index 142c6020cec7..62830a43d959 100644
--- a/drivers/crypto/s5p-sss.c
+++ b/drivers/crypto/s5p-sss.c
@@ -
The dma_map_sg() function returns zero on error and positive values on
success. We want to return -ENOMEM on failure here and zero on success.
Fixes: 2f47d5804311 ("crypto: chelsio - Move DMA un/mapping to chcr from lld
cxgb4 driver")
Signed-off-by: Dan Carpenter <dan.carpen.
Reviewed-by: Dan Carpenter <dan.carpen...@oracle.com>
regards,
dan carpenter
On Thu, Nov 09, 2017 at 08:27:28AM +0200, Gilad Ben-Yossef wrote:
> On Tue, Nov 7, 2017 at 12:43 PM, Dan Carpenter <dan.carpen...@oracle.com>
> wrote:
> > On Tue, Nov 07, 2017 at 09:40:02AM +, Gilad Ben-Yossef wrote:
> >> --- a/drivers/staging/ccree/ssi_pm.c
>
in_t(unsigned int, small, CHCR_SRC_SG_SIZE);
564 walk->sgl->len0 = cpu_to_be32(sgmin);
regards,
dan carpenter
(blocksize - keylen));
> @@ -539,10 +539,10 @@ ssi_get_plain_hmac_key(struct crypto_aead *tfm, const
> u8 *key, unsigned int keyl
> }
>
> rc = send_request(ctx->drvdata, _req, desc, idx, 0);
> - if (unlikely(rc != 0))
> +
struct ssi_drvdata *drvdata =
> + (struct ssi_drvdata *)dev_get_drvdata(dev);
No need to cast:
struct ssi_drvdata *drvdata = dev_get_drvdata(dev);
regards,
dan carpenter
areq_ctx->req_authsize),
> -(size_to_skip + req->cryptlen),
> - SSI_SG_FROM_BUF);
> - }
> + cc_copy_mac(dev, req, SSI_SG_FROM_BUF);
> }
regards,
dan carpenter
(areq->cryptlen +
> - areq_ctx->dst_offset +
> - ctx->authsize),
> +areq_ctx->dst_sgl, loc,
> +(loc + ctx->authsize),
> SSI_SG_FROM_BUF);
> }
regards,
dan carpenter
to be strict about the one
thing per patch.
> + iv_size_to_authenc, is_last,
> + _ctx->assoc.mlli_nents);
> areq_ctx->assoc_buff_type = SSI_DMA_BUF_MLLI;
> }
>
regards,
dan carpenter
wn error messages if needed and return directly. If there is no
cleanup then there is no need for a goto.
Anyway, that's not related to this patch. Just resend it with
goto post_clk_err: in the v2.
regards,
dan carpenter
ent_mask(_dev->dev, dma_mask);
> + if (!rc)
> + break;
The indenting is messed up.
> + }
> + dma_mask >>= 1;
> + }
regards,
dan carpenter
ctx->iv, info, ivsize);
We need to check if kmalloc() fails.
regards,
dan carpenter
+= crypto_aead_ivsize(tfm);
> + if (areq_ctx->is_gcm4543)
> + size_to_skip += crypto_aead_ivsize(tfm);
>
> ssi_buffer_mgr_copy_scatterlist_portion(
> dev, areq_ctx->backup_mac, req->src,
But then ssi_buffer_mgr_copy_scatterlist_portion() is still not indented
correctly.
regards,
dan carpenter
t ssi_request_mgr_handle {
> dma_addr_t dummy_comp_buff_dma;
> struct cc_hw_desc monitor_desc;
>
> - volatile unsigned long monitor_lock;
> + unsigned long monitor_lock;
The variable seems unused. Try deleting it instead.
regards,
dan carpenter
unsigned long size) {};
Could you put the {} on the next line? Also there is no need for the
semi-colon after the end of a function.
This is a style thing, so if you want to do it in a follow on patch
that's fine
regards,
dan carpenter
Looks good. Thanks!
regards,
dan carpenter
, it sometimes still stresses me out. It's
like you're disagreeing with the original author and the reviewers are
disagreeing with you and everyone's trying to be nice about it but
patches are fundamentally points of disagreement and that's stress.
regards,
dan carpenter
I've tried
to review locking bugs to see if single returns prevent future
programmers from introducing new error paths which don't unlock. They
don't really help...
regards,
dan carpenter
,
dan carpenter
Google for how to send a v2 patch.
https://www.google.com/search?q=how+to+send+a+v2+patch
https://kernelnewbies.org/FirstKernelPatch
regards,
dan carpenter
:
1087 /*Finish current request */
1088 stm32_hash_finish_req(hdev->req, err);
^^^
Never initialized.
1089
1090 return IRQ_HANDLED;
1091 }
regards,
dan carpenter
On Fri, Jul 28, 2017 at 09:59:41AM +0530, Suniel Mahesh wrote:
> On Friday 28 July 2017 01:18 AM, Dan Carpenter wrote:
> > On Thu, Jul 27, 2017 at 05:27:33PM +0300, Gilad Ben-Yossef wrote:
> >> + new_drvdata->cc_base = devm_iorem
the
bottom of the function or jump to a different file.
regards,
dan carpenter
eels like -EINVAL
with a WARN_ON_ONCE() message would be better but I don't really
understand this code.
163 u_ctx = ERR_PTR(-ENOMEM);
164 goto out;
165 }
166 u_ctx->lldi = *lld;
167 out:
168 return u_ctx;
a:
285 kzfree(work_data);
286 kpp_request_complete(req, status);
287 }
regards,
dan carpenter
ctx->base.needs_inv = true;
893 break;
894 }
895 }
896
897 return 0;
898 }
regards,
dan carpenter
memset(out, 0, HASH_LEN_SIZE);
}
out += HASH_LEN_SIZE;
The ->import() function has a similar snippet.
regards,
dan carpenter
devices have a clock associated with CCREE */
This is not related. Do unrelated things in different patches. This
typo was introduced in an earlier patch. There are a couple ways in git
to edit previous patches.
> goto out;
>
> rc = clk_prepare_enable(clk);
regards,
dan carpenter
vdata *drvdata)
{
struct clk *clk = drvdata->clk;
int rc;
if (IS_ERR(clk)) {
/* Not all devices have a clock associated with CCREE */
return 0;
}
rc = clk_prepare_enable(clk);
if (rc)
return rc;
return 0;
}
regards,
dan carpenter
rrent staging-next which
> is d06838de4a015c7c4844ad3fcf63ad5e2c17b234 so it will conflict with
> the coding style clean up patches from Jhin-Ming if you take them.
>
> If you wish me to merge this patch set on top those just let me know.
>
Yes. Those are fine and will be merged most likely. It's strictly
first in, first out.
regards,
dan carpenter
On Tue, Jun 20, 2017 at 10:51:58PM +0800, Jhih-Ming Huang wrote:
>
> Hi,
>
> This patch fix all coding style error in driver/staging/ccree/ssi_aead.c.
Much better. Thanks!
regards,
dan carpenter
{
Remove the other space as well. I looked ahead in the series so I see
that you do it later, but it should be done here.
regards,
dan carpenter
On Tue, Jun 20, 2017 at 01:20:59PM +0800, Jhih-Ming Huang wrote:
> From: Jhih-Ming Hunag <fbihjme...@gmail.com>
>
> Fixed 'ERROR: spaces required around that'
>
You're breaking the patches up in a bad way. This one should be
combined with the previous patch.
regards,
dan carpenter
gt; driver/staging/ccree/ssi_aead.c from 54 errors to 0 error.
You could put this into the cover letter. When we put this into the
final git log we don't see the series only individual patches.
>
> The first patch fixed 'ERROR: space required after that'.
>
This patch fixes ...
regards,
dan carpenter
We want to return negative error codes here, but we're accidentally
propogating the "true" return from dma_mapping_error().
Fixes: 14fa93cdcd9b ("crypto: cavium - Add support for CNN55XX adapters.")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/dr
We forgot to set the error code on this path so it could result in
returning NULL which leads to a NULL dereference.
Fixes: db6c43bd2132 ("crypto: KEYS: convert public key and digsig asym to the
akcipher api")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
---
v2: Sty
We forgot to set the error code on this path so it could result in
returning NULL which leads to a NULL dereference.
Fixes: db6c43bd2132 ("crypto: KEYS: convert public key and digsig asym to the
akcipher api")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
dif
We checked (nbytes < bsize) inside the loops so it's not possible to hit
the "goto done;" here. This code is cut and paste from other slightly
different loops where we don't have the check inside the loop.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --gi
We recently added some new locking but missed the unlocks on these
error paths in sha512_ctx_mgr_submit().
Fixes: c459bd7beda0 ("crypto: sha512-mb - Protect sha512 mb ctx mgr access")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/arch/x86/crypto/sha512-m
On Thu, Apr 13, 2017 at 08:37:50PM +0530, Harsh Jain wrote:
> On Thu, Apr 13, 2017 at 8:20 PM, Christophe JAILLET
> <christophe.jail...@wanadoo.fr> wrote:
> > Le 13/04/2017 à 16:04, Dan Carpenter a écrit :
> >>
> >> On Thu, Apr 13, 2017 at 02:14:
g style error handling is that "base_hash" wasn't allocated so
this will oops for both NULL and error pointers.
regards,
dan carpenter
chcr_free_shash(base_hash);
Ah... Ok. Fine, but redo the first patch anyway because it shouldn't
ever be NULL.
regards,
dan carpenter
h(>lock);
254 return;
255 }
regards,
dan carpenter
On Sat, Mar 18, 2017 at 11:24:34AM +0100, walter harms wrote:
>
>
> Am 17.03.2017 21:46, schrieb Dan Carpenter:
> > There is a typo here. It should be "stats" instead of "state". The
> > impact is that we clear 224 bytes instead of 80 and
There is a typo here. It should be "stats" instead of "state". The
impact is that we clear 224 bytes instead of 80 and we zero out memory
that we shouldn't.
Fixes: 09ae5d37e093 ("crypto: zip - Add Compression/Decompression statistics")
Signed-off-by: Dan Carpe
/* Convey DOWN to PF */
899 if (cptvf_send_vf_down(cptvf)) {
^
Oops inside function.
900 dev_err(>dev, "PF not responding to DOWN msg");
901 } else {
regards,
dan carpenter
nfo || !cptvf) {
^
Check is too late.
334 dev_err(>dev, "Input params are incorrect for
post processing\n");
335 return;
regards,
dan carpenter
82 cpt_write_csr64(cpt->reg_base, CPTX_PF_QX_CTL(0, q),
pf_qx_ctl.u);
83 dev_dbg(dev, "VF %d TYPE %s", q, (mcode[grp].is_ae ? "AE" :
"SE"));
84
85 return mcode[grp].is_ae ? AE_TYPES : SE_TYPES;
86 }
regards,
dan carpenter
w */
2344 crypto_free_shash(ctx->shash->tfm);
2345 kfree(ctx->shash);
2346 return ret;
2347 }
regards,
dan carpenter
_param = dd;
675
676 atmel_sha_write_ctrl(dd, 1);
677
regards,
dan carpenter
Ping?
regards,
dan carpenter
On Thu, Dec 01, 2016 at 11:48:58PM +0300, Dan Carpenter wrote:
> Hello Harsh Jain,
>
> This is a semi-automatic email about new static checker warnings.
>
> The patch 2debd3325e55: "crypto: chcr - Add AEAD algos." from Nov 29,
> 2016, l
pr_err("chcr : %s : No crypto device.\n", __func__);
2462 return -ENXIO;
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
We added some new locking but forgot to unlock on error.
Fixes: 57127645d79d ("s390/zcrypt: Introduce new SHA-512 based Pseudo Random
Generator.")
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/arch/s390/crypto/prng.c b/arch/s390/crypto/prng.c
index 9cc050f
return 0;
810
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
utex_unlock(>sb_mutex);
59
60 /* Wait for KSB entries to become available */
61 if (wait_event_interruptible(ccp->sb_queue,
ccp->sb_avail))
62 return 0;
63 }
64 }
regards,
dan carpenter
8 spage = sg_page(sg);
^^^
Unchecked dereference inside function.
379 get_page(spage);
380 page_len = min(sg->length, count);
regards,
dan carpenter
--
To unsubscribe from this list: send the lin
igned-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/crypto/nx/nx.c b/drivers/crypto/nx/nx.c
index 0794f1c..42f0f22 100644
--- a/drivers/crypto/nx/nx.c
+++ b/drivers/crypto/nx/nx.c
@@ -392,7 +392,7 @@ static void nx_of_update_msc(struct device *dev,
On Thu, Jun 30, 2016 at 01:42:19PM -0700, Tim Chen wrote:
> On Wed, 2016-06-29 at 10:05 -0700, H. Peter Anvin wrote:
> > On 06/29/16 07:42, Dan Carpenter wrote:
> > >
> > > >
> > > > >
> > > > > and | behave basically the same here b
The difference between | and || is that || has ordering constraints.
It's from the C standard, and not the compiler version.
regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majord...@vger.kernel.org
More majo
On Wed, Jun 29, 2016 at 10:05:53AM -0700, H. Peter Anvin wrote:
> On 06/29/16 07:42, Dan Carpenter wrote:
> > || and | behave basically the same here but || is intended. It causes a
> > static checker warning to mix up bitwise and logical operations.
> >
> > S
The "goto out;" line isn't indented far enough.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 6ef7815..117f19e 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -629,7 +629,7 @@ static void test_mb_ahash_speed(
|| and | behave basically the same here but || was intended. It causes
a static checker warning when we mix up logical and bitwise operations.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/arch/x86/crypto/sha1-mb/sha1_mb.c
b/arch/x86/crypto/sha1-mb/sha1_mb.c
|| and | behave basically the same here but || is intended. It causes a
static checker warning to mix up bitwise and logical operations.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/arch/x86/crypto/sha256-mb/sha256_mb.c
b/arch/x86/crypto/sha256-mb/sha256_mb.c
We accidentally return PTR_ERR(NULL) which is success but we should
return -ENOMEM.
Fixes: 355912852115 ('crypto: drbg - use CTR AES instead of ECB AES')
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/crypto/drbg.c b/crypto/drbg.c
index ded8638..6872d15 100644
--- a/
On Wed, May 18, 2016 at 01:42:36PM +0300, Peter Ujfalusi wrote:
> On 05/18/16 13:39, Dan Carpenter wrote:
> > This if statement is reversed so we end up either leaking or Oopsing on
> > error.
>
> Oops, sorry for that.
> Probably the other omap-* crypto drivers have the
This if statement is reversed so we end up either leaking or Oopsing on
error.
Fixes: dbe246209bc1 ('crypto: omap-sham - Use dma_request_chan() for requesting
DMA channel')
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/crypto/omap-sham.c b/drivers/crypt
he MXC SCC')
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/crypto/mxc-scc.c b/drivers/crypto/mxc-scc.c
index 9b348a7..ff383ef 100644
--- a/drivers/crypto/mxc-scc.c
+++ b/drivers/crypto/mxc-scc.c
@@ -616,7 +616,7 @@ static struct mxc_scc_crypto_tmpl *scc_crypto_algs
->src_nents and ->dst_nents are unsigned so they can't be less than
zero. I fixed this by introducing a temporary "nents" variable.
Fixes: d293b640ebd5 ('crypto: mxc-scc - add basic driver for the MXC SCC')
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --gi
creq->cache[] is an array inside the struct, it's not a pointer and it
can't be NULL.
Signed-off-by: Dan Carpenter <dan.carpen...@oracle.com>
diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c
index 7ca2e0f..7a5058d 100644
--- a/drivers/crypto/marvell/hash.c
+++
1 - 100 of 137 matches
Mail list logo