Re: [PATCH i-g-t 1/3] Avoid truncate string in __igt_lsof_fds

From: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
To: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: igt-dev@lists.freedesktop.org, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH i-g-t 1/3] Avoid truncate string in __igt_lsof_fds
Date: Mon, 4 Jun 2018 22:45:27 -0300	[thread overview]
Message-ID: <20180605014527.petnzluv3mz3ccvg@smtp.gmail.com> (raw)
In-Reply-To: <20180604104037.GG16222@ahiler-desk1.ger.corp.intel.com>

Hi Arkadiusz,

First of all, thanks for the review.

I believe that I understood all the comments you made in all of the
patches. I will send the V2 soon.

Thanks!

Best Regards
Rodrigo Siqueira

On 06/04, Arkadiusz Hiler wrote:
> On Tue, May 29, 2018 at 09:46:38PM -0300, Rodrigo Siqueira wrote:
> > Note that 'proc_path' parameter in __igt_lsof_fds receives a string
> > which was initialized with the size of PATH_MAX and the local variable
> > 'path' has the same size, but it also have to append: '/', '\0', and the
> > directory name. This situation caused the warning described below.
> > 
> > warning: ‘%s’ directive output may be truncated writing up to 255 bytes
> > into a region of size between 0 and 4095 [-Wformat-truncation=]
> > snprintf(path, sizeof(path), "%s/%s", proc_path, d->d_name);
> > note: ‘snprintf’ output between 2 and 4352 bytes into a destination of
> > size 4096 [..]
> > 
> > This patch fix the above problem.
> > 
> > Signed-off-by: Rodrigo Siqueira <rodrigosiqueiramelo@gmail.com>
> > ---
> >  lib/igt_aux.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> > index acafb713..5dd2761e 100644
> > --- a/lib/igt_aux.c
> > +++ b/lib/igt_aux.c
> > @@ -1425,7 +1425,7 @@ __igt_lsof_fds(proc_t *proc_info, int *state, char *proc_path, const char *dir)
> >  {
> >  	struct dirent *d;
> >  	struct stat st;
> > -	char path[PATH_MAX];
> > +	char path[PATH_MAX + NAME_MAX + 2]; // 1 byte for '/' and another for '\0'
> 
> Hey,
> 
> First of, thanks for looking at the new warnings. We definitely have to
> fix those :-)
> 
> A couple of lines down we have lstat(path, &st), which will return -1
> with errno == ENAMETOOLONG if we go over PATH_MAX, so this does not feel
> right, especially that we are not checking for that.
> 
> Digging deeper, __igt_lsof_fds() is used only inside __igt_lsof(), which
> is the reason for such a high estimate.
> 
> It uses char path[PATH_MAX], which is the main contributing component,
> but it contains at most strlen("/proc/%d/cwd")+1 where "%d" is
> CEILING(LOG_10(INT_MAX)).
> 
> Limiting size of path there to sensible upper estimation should make us
> fit in PATH_MAX in __igt_lsof_fds (or even less).
> 
> -- 
> Cheers,
> Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx