Re: [PATCH] xfs: Use scnprintf() for avoiding potential buffer overflow

Dave Chinner
2020-03-11T22:09:21Z

On Wed, Mar 11, 2020 at 10:35:52AM +0100, Takashi Iwai wrote:
> Since snprintf() returns the would-be-output size instead of the
> actual output size, the succeeding calls may go beyond the given
> buffer limit. Fix it by replacing with scnprintf().
>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> fs/xfs/xfs_stats.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)

what about all the other calls to snprintf() in fs/xfs/xfs_sysfs.c
and fs/xfs/xfs_error.c that return the "would be written" length to
their callers? i.e. we can return a length longer than the buffer
provided to the callers...

Aren't they all broken, too?

A quick survey of random snprintf() calls shows there's an abundance
of callers that do not check the return value of snprintf for
overflow when outputting stuff to proc/sysfs files. This seems like
a case of "snprintf() considered harmful, s/snprintf/scnprintf/
kernel wide, remove snprintf()"...

Cheers,

Dave,
--
Dave Chinner
david@fromorbit.com