On 22.10.2018 12.37, Thierry Reding wrote:
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.
tag is always 32 characters. So I'll just update the code to check that
'size' parameter always equals to 32 to avoid any further complexity here.
-Timo
