Hi Gilad,
On Sun, Jan 26, 2020 at 2:38 PM Gilad Ben-Yossef <gilad@xxxxxxxxxxxxx> 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 <gilad@xxxxxxxxxxxxx>
> Reported-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
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 -- geert@xxxxxxxxxxxxxx
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
