On Fri, May 25, 2018 at 11:05:47AM +0800, Anand Jain wrote:
> Balance arg info is an important information to be reviewed for the
> system audit. So this patch adds them to the kernel log.
>
> Example:
> ->btrfs bal start -f -mprofiles=raid1,convert=single,soft
> -dlimit=10..20,usage=50 /btrfs
>
> kernel: BTRFS info (device sdb): balance: start -f -dusage=50,limit=10..20
> -msoft,profiles=raid1,convert=single -ssoft,profiles=raid1,convert=single
>
> Signed-off-by: Anand Jain <anand.j...@oracle.com>
> ---
> v4->v4.1b: Rename last missed sz to size_buf.
Please send a full version instead of the incremental bumps to
individual patches. I try to take the last version but am never sure
what exactly gets merged.
> +static u32 describe_balance_args(struct btrfs_balance_args *bargs, char *buf,
> + u32 size_buf)
> +{
> + char *bp = buf;
> + u64 flags = bargs->flags;
> + char tmp_buf[128];
> +
> + if (!flags)
> + return 0;
> +
> + if (flags & BTRFS_BALANCE_ARGS_SOFT)
> + bp += snprintf(bp, buf - bp + size_buf, "soft,");
I have some suspicion that this snprintf conscturct is not safe when the
value of 'buf - bp + size_buf' becomes accidentally negative. A quick
test showed that a sequence of snprintf and incremented bp does not stop
writing when the size_buf limit is hit and happily overwrites the
memory.
We'd have to check that the negative value is never passed to snprintf.
The patches are potponed until the issue is resolved, either to verify
that it's a false alert or the bounds are correctly checked.
The bug should not be in the original code as the buffer is known to be
large enough for all the printed bg flags.
I'm sorry to delay the patches, the log messages are useful, but I want
to be sure that there are no random memory overwrites introduced.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
