| Message ID | 20231221080957.1081077-1-clg@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | vfio/iommufd: Remove the use of stat() to check file existence | expand |
>-----Original Message----- >From: Cédric Le Goater <clg@redhat.com> >Sent: Thursday, December 21, 2023 4:10 PM >Subject: [PATCH] vfio/iommufd: Remove the use of stat() to check file >existence > >Using stat() before opening a file or a directory can lead to a >time-of-check to time-of-use (TOCTOU) filesystem race, which is >reported by coverity as a Security best practices violations. The >sequence could be replaced by open and fdopendir but it doesn't add >much in this case. Simply use opendir to avoid the race. > >Fixes: CID 1531551 >Signed-off-by: Cédric Le Goater <clg@redhat.com> Thanks for fixing, Reviewed-by: Zhenzhong Duan <Zhenzhong.duan@intel.com> BRs. Zhenzhong >--- > hw/vfio/iommufd.c | 6 ------ > 1 file changed, 6 deletions(-) > >diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c >index >d4c586e842def8f04d3a914843f5eece2c75ea30..9bfddc1360895413176a9f >170e29e89027384a66 100644 >--- a/hw/vfio/iommufd.c >+++ b/hw/vfio/iommufd.c >@@ -121,17 +121,11 @@ static int iommufd_cdev_getfd(const char >*sysfs_path, Error **errp) > DIR *dir = NULL; > struct dirent *dent; > gchar *contents; >- struct stat st; > gsize length; > int major, minor; > dev_t vfio_devt; > > path = g_strdup_printf("%s/vfio-dev", sysfs_path); >- if (stat(path, &st) < 0) { >- error_setg_errno(errp, errno, "no such host device"); >- goto out_free_path; >- } >- > dir = opendir(path); > if (!dir) { > error_setg_errno(errp, errno, "couldn't open directory %s", path); >-- >2.43.0
Hello Zhenzhong On 12/21/23 09:55, Duan, Zhenzhong wrote: > > >> -----Original Message----- >> From: Cédric Le Goater <clg@redhat.com> >> Sent: Thursday, December 21, 2023 4:10 PM >> Subject: [PATCH] vfio/iommufd: Remove the use of stat() to check file >> existence >> >> Using stat() before opening a file or a directory can lead to a >> time-of-check to time-of-use (TOCTOU) filesystem race, which is >> reported by coverity as a Security best practices violations. The >> sequence could be replaced by open and fdopendir but it doesn't add >> much in this case. Simply use opendir to avoid the race. >> >> Fixes: CID 1531551 >> Signed-off-by: Cédric Le Goater <clg@redhat.com> > > Thanks for fixing, Reviewed-by: Zhenzhong Duan <Zhenzhong.duan@intel.com> It seems that tools like b4 need the R-b tag, and probably other tags, to be at the beginning of a new line. So, just repeating : Reviewed-by: Zhenzhong Duan <Zhenzhong.duan@intel.com> Thanks, C. > > BRs. > Zhenzhong > >> --- >> hw/vfio/iommufd.c | 6 ------ >> 1 file changed, 6 deletions(-) >> >> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c >> index >> d4c586e842def8f04d3a914843f5eece2c75ea30..9bfddc1360895413176a9f >> 170e29e89027384a66 100644 >> --- a/hw/vfio/iommufd.c >> +++ b/hw/vfio/iommufd.c >> @@ -121,17 +121,11 @@ static int iommufd_cdev_getfd(const char >> *sysfs_path, Error **errp) >> DIR *dir = NULL; >> struct dirent *dent; >> gchar *contents; >> - struct stat st; >> gsize length; >> int major, minor; >> dev_t vfio_devt; >> >> path = g_strdup_printf("%s/vfio-dev", sysfs_path); >> - if (stat(path, &st) < 0) { >> - error_setg_errno(errp, errno, "no such host device"); >> - goto out_free_path; >> - } >> - >> dir = opendir(path); >> if (!dir) { >> error_setg_errno(errp, errno, "couldn't open directory %s", path); >> -- >> 2.43.0 >
>-----Original Message----- >From: Cédric Le Goater <clg@redhat.com> >Sent: Thursday, December 21, 2023 5:16 PM >Subject: Re: [PATCH] vfio/iommufd: Remove the use of stat() to check file >existence > >Hello Zhenzhong > >On 12/21/23 09:55, Duan, Zhenzhong wrote: >> >> >>> -----Original Message----- >>> From: Cédric Le Goater <clg@redhat.com> >>> Sent: Thursday, December 21, 2023 4:10 PM >>> Subject: [PATCH] vfio/iommufd: Remove the use of stat() to check file >>> existence >>> >>> Using stat() before opening a file or a directory can lead to a >>> time-of-check to time-of-use (TOCTOU) filesystem race, which is >>> reported by coverity as a Security best practices violations. The >>> sequence could be replaced by open and fdopendir but it doesn't add >>> much in this case. Simply use opendir to avoid the race. >>> >>> Fixes: CID 1531551 >>> Signed-off-by: Cédric Le Goater <clg@redhat.com> >> >> Thanks for fixing, Reviewed-by: Zhenzhong Duan ><Zhenzhong.duan@intel.com> > >It seems that tools like b4 need the R-b tag, and probably other >tags, to be at the beginning of a new line. So, just repeating : > >Reviewed-by: Zhenzhong Duan <Zhenzhong.duan@intel.com> Got it, will follow that rule in the future. Thanks Zhenzhong
On 12/21/23 09:09, Cédric Le Goater wrote: > Using stat() before opening a file or a directory can lead to a > time-of-check to time-of-use (TOCTOU) filesystem race, which is > reported by coverity as a Security best practices violations. The > sequence could be replaced by open and fdopendir but it doesn't add > much in this case. Simply use opendir to avoid the race. > > Fixes: CID 1531551 > Signed-off-by: Cédric Le Goater <clg@redhat.com> Applied to vfio-next. Thanks, C.
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c index d4c586e842def8f04d3a914843f5eece2c75ea30..9bfddc1360895413176a9f170e29e89027384a66 100644 --- a/hw/vfio/iommufd.c +++ b/hw/vfio/iommufd.c @@ -121,17 +121,11 @@ static int iommufd_cdev_getfd(const char *sysfs_path, Error **errp) DIR *dir = NULL; struct dirent *dent; gchar *contents; - struct stat st; gsize length; int major, minor; dev_t vfio_devt; path = g_strdup_printf("%s/vfio-dev", sysfs_path); - if (stat(path, &st) < 0) { - error_setg_errno(errp, errno, "no such host device"); - goto out_free_path; - } - dir = opendir(path); if (!dir) { error_setg_errno(errp, errno, "couldn't open directory %s", path);
Using stat() before opening a file or a directory can lead to a time-of-check to time-of-use (TOCTOU) filesystem race, which is reported by coverity as a Security best practices violations. The sequence could be replaced by open and fdopendir but it doesn't add much in this case. Simply use opendir to avoid the race. Fixes: CID 1531551 Signed-off-by: Cédric Le Goater <clg@redhat.com> --- hw/vfio/iommufd.c | 6 ------ 1 file changed, 6 deletions(-)