On Fri, Oct 19, 2018 at 01:07:19PM +0100, Jon Hunter wrote: > > > On 18/10/2018 18:55, Timo Alho wrote: > > Last two characters of the version tag that is 32 bytes long were > > stripped out. > > > > Signed-off-by: Timo Alho <talho@xxxxxxxxxx> > > --- > > drivers/firmware/tegra/bpmp.c | 11 ++++++----- > > 1 file changed, 6 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/firmware/tegra/bpmp.c b/drivers/firmware/tegra/bpmp.c > > index 79859ab..a7d8954 100644 > > --- a/drivers/firmware/tegra/bpmp.c > > +++ b/drivers/firmware/tegra/bpmp.c > > @@ -558,8 +558,9 @@ static int tegra_bpmp_get_firmware_tag(struct tegra_bpmp *bpmp, char *tag, > > dma_addr_t phys; > > void *virt; > > int err; > > + const size_t tag_sz = 32; > > Why not a #define for this? > > > > > - virt = dma_alloc_coherent(bpmp->dev, MSG_DATA_MIN_SZ, &phys, > > + virt = dma_alloc_coherent(bpmp->dev, tag_sz, &phys, > > GFP_KERNEL | GFP_DMA32); > > if (!virt) > > return -ENOMEM; > > @@ -577,9 +578,9 @@ static int tegra_bpmp_get_firmware_tag(struct tegra_bpmp *bpmp, char *tag, > > local_irq_restore(flags); > > > > if (err == 0) > > - strlcpy(tag, virt, size); > > + memcpy(tag, virt, min(size, tag_sz)); > > Should it be an error if size is less than 32? Otherwise characters > could be silently stripped. Is the tag guaranteed to always be 32 characters? If it was possible for it to be less, we'd now be copying out uninitialized characters, right? I think dma_alloc_coherent() zeroes memory, so this should be safe, but we should double-check to make sure we're not potentially leaking data here. Thierry
Attachment:
signature.asc
Description: PGP signature
