| Message ID | 03a936e0c4bca03f89e731d2cacaa3624a10147f.1527639529.git.rodrigosiqueiramelo@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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).
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
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' char *fd_lnk; /* default fds or kernel threads */
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(-)