Re: [PATCH kvmtool 1/3] builtin-run: Fix warning when resolving path

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Will,

Thanks for taking the time to review this patch,

On Fri, Feb 01, 2019 at 04:27:16PM +0000, Will Deacon wrote:
> On Thu, Jan 17, 2019 at 01:40:12PM +0100, Anisse Astier wrote:
> > GCC 8.2 gives this warning:
> > 
> > builtin-run.c: In function ‘kvm_run_write_sandbox_cmd.isra.1’:
> > builtin-run.c:417:28: error: ‘%s’ directive output may be truncated writing up to 4095 bytes into a region of size 4091 [-Werror=format-truncation=]
> >    snprintf(dst, len, "/host%s", resolved_path);
> >                             ^~   ~~~~~~~~~~~~~
> > 
> > It's because it understands that len is PATH_MAX, the same as
> > resolved_path's size. This patch handles the case where the string is
> > truncated, and fixes the warning.
> > 
> > Signed-off-by: Anisse Astier <aastier@xxxxxxxxxx>
> > ---
> >  builtin-run.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/builtin-run.c b/builtin-run.c
> > index 443c10b..32d8aef 100644
> > --- a/builtin-run.c
> > +++ b/builtin-run.c
> > @@ -414,7 +414,9 @@ static void resolve_program(const char *src, char *dst, size_t len)
> >  		if (!realpath(src, resolved_path))
> >  			die("Unable to resolve program %s: %s\n", src, strerror(errno));
> >  
> > -		snprintf(dst, len, "/host%s", resolved_path);
> > +		if (snprintf(dst, len, "/host%s", resolved_path) >= (int)len - 1)
> > +			die("Pathname too long: %s -> %s\n", src, resolved_path);
> 
> Why do we need the '- 1' here?
> 
> Will

Good catch, I think it's not needed in this case. I'll send an updated
patch.

Anisse



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux