Deal gracefully with the event of being handed a scatterlist
which is shorter than expected.
This mitigates a crash in some cases of crashes due to
attempt to map empty (but not NULL) scatterlists with none
zero lengths.
This is an interim patch, to help diagnoze the issue, not
intended for mainline in its current form as of yet.
Signed-off-by: Gilad Ben-Yossef <[email protected]>
Reported-by: Geert Uytterhoeven <[email protected]>
---
drivers/crypto/ccree/cc_buffer_mgr.c | 30 +++++++++++++++++++++++++---
1 file changed, 27 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/ccree/cc_buffer_mgr.c b/drivers/crypto/ccree/cc_buffer_mgr.c
index a72586eccd81..9667a2630c66 100644
--- a/drivers/crypto/ccree/cc_buffer_mgr.c
+++ b/drivers/crypto/ccree/cc_buffer_mgr.c
@@ -286,10 +286,32 @@ static void cc_add_sg_entry(struct device *dev, struct buffer_array *sgl_data,
sgl_data->num_of_buffers++;
}
+static unsigned int cc_sg_trunc_len(struct scatterlist *sg, unsigned int len)
+{
+ unsigned int total;
+
+ if (!len)
+ return 0;
+
+ for (total = 0; sg; sg = sg_next(sg)) {
+ total += sg->length;
+ if (total >= len) {
+ total = len;
+ break;
+ }
+ }
+
+ return total;
+}
+
static int cc_map_sg(struct device *dev, struct scatterlist *sg,
unsigned int nbytes, int direction, u32 *nents,
u32 max_sg_nents, u32 *lbytes, u32 *mapped_nents)
{
+ int ret;
+
+ nbytes = cc_sg_trunc_len(sg, nbytes);
+
if (sg_is_last(sg)) {
/* One entry only case -set to DLLI */
if (dma_map_sg(dev, sg, 1, direction) != 1) {
@@ -313,12 +335,14 @@ static int cc_map_sg(struct device *dev, struct scatterlist *sg,
/* In case of mmu the number of mapped nents might
* be changed from the original sgl nents
*/
- *mapped_nents = dma_map_sg(dev, sg, *nents, direction);
- if (*mapped_nents == 0) {
+ ret = dma_map_sg(dev, sg, *nents, direction);
+ if (dma_mapping_error(dev, ret)) {
*nents = 0;
- dev_err(dev, "dma_map_sg() sg buffer failed\n");
+ dev_err(dev, "dma_map_sg() sg buffer failed %d\n", ret);
return -ENOMEM;
}
+
+ *mapped_nents = ret;
}
return 0;
--
2.23.0
Hi Gilad,
On Sun, Jan 26, 2020 at 2:38 PM Gilad Ben-Yossef <[email protected]> wrote:
> Deal gracefully with the event of being handed a scatterlist
> which is shorter than expected.
>
> This mitigates a crash in some cases of crashes due to
> attempt to map empty (but not NULL) scatterlists with none
> zero lengths.
>
> This is an interim patch, to help diagnoze the issue, not
> intended for mainline in its current form as of yet.
>
> Signed-off-by: Gilad Ben-Yossef <[email protected]>
> Reported-by: Geert Uytterhoeven <[email protected]>
Thanks for your patch!
Unfortunately this doesn't make a difference, as ...
> --- a/drivers/crypto/ccree/cc_buffer_mgr.c
> +++ b/drivers/crypto/ccree/cc_buffer_mgr.c
> @@ -286,10 +286,32 @@ static void cc_add_sg_entry(struct device *dev, struct buffer_array *sgl_data,
> sgl_data->num_of_buffers++;
> }
>
> +static unsigned int cc_sg_trunc_len(struct scatterlist *sg, unsigned int len)
> +{
> + unsigned int total;
> +
> + if (!len)
> + return 0;
> +
> + for (total = 0; sg; sg = sg_next(sg)) {
> + total += sg->length;
> + if (total >= len) {
> + total = len;
> + break;
> + }
> + }
> +
> + return total;
> +}
> +
> static int cc_map_sg(struct device *dev, struct scatterlist *sg,
> unsigned int nbytes, int direction, u32 *nents,
> u32 max_sg_nents, u32 *lbytes, u32 *mapped_nents)
> {
> + int ret;
> +
> + nbytes = cc_sg_trunc_len(sg, nbytes);
> +
> if (sg_is_last(sg)) {
(1) this branch is taken, and not the else below,
(2) nothing acts upon detecting nbytes = 0.
With extra debug print:
cc_map_sg: nbytes = 0, first branch taken
> /* One entry only case -set to DLLI */
> if (dma_map_sg(dev, sg, 1, direction) != 1) {
> @@ -313,12 +335,14 @@ static int cc_map_sg(struct device *dev, struct scatterlist *sg,
> /* In case of mmu the number of mapped nents might
> * be changed from the original sgl nents
> */
> - *mapped_nents = dma_map_sg(dev, sg, *nents, direction);
> - if (*mapped_nents == 0) {
> + ret = dma_map_sg(dev, sg, *nents, direction);
> + if (dma_mapping_error(dev, ret)) {
> *nents = 0;
> - dev_err(dev, "dma_map_sg() sg buffer failed\n");
> + dev_err(dev, "dma_map_sg() sg buffer failed %d\n", ret);
> return -ENOMEM;
> }
> +
> + *mapped_nents = ret;
> }
>
> return 0;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Mon, Jan 27, 2020 at 10:03 AM Geert Uytterhoeven
<[email protected]> wrote:
>
> Hi Gilad,
>
> On Sun, Jan 26, 2020 at 2:38 PM Gilad Ben-Yossef <[email protected]> wrote:
> > Deal gracefully with the event of being handed a scatterlist
> > which is shorter than expected.
> >
> > This mitigates a crash in some cases of crashes due to
> > attempt to map empty (but not NULL) scatterlists with none
> > zero lengths.
> >
> > This is an interim patch, to help diagnoze the issue, not
> > intended for mainline in its current form as of yet.
> >
> > Signed-off-by: Gilad Ben-Yossef <[email protected]>
> > Reported-by: Geert Uytterhoeven <[email protected]>
>
> Thanks for your patch!
>
> Unfortunately this doesn't make a difference, as ...
OK, so this is a different case than the one I am seeing but similar
in the sense that we get a scatterlist with
a NULL first buffer, which aead.h says we shouldn't... Oh, well.
Sorry, still waiting to get my R-Car board back. Please try the patch
I'm about to send and see if it is better.
Thanks,
Gilad
--
Gilad Ben-Yossef
Chief Coffee Drinker
values of β will give rise to dom!