util/vfio-helpers.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
When _FORTIFY_SOURCE=2, glibc version is 2.35, and GCC version is
12.1.0, the compiler complains as follows:
In file included from /usr/include/features.h:490,
from /usr/include/bits/libc-header-start.h:33,
from /usr/include/stdint.h:26,
from /usr/lib/gcc/aarch64-unknown-linux-gnu/12.1.0/include/stdint.h:9,
from /home/alarm/q/var/qemu/include/qemu/osdep.h:94,
from ../util/vfio-helpers.c:13:
In function 'readlink',
inlined from 'sysfs_find_group_file' at ../util/vfio-helpers.c:116:9,
inlined from 'qemu_vfio_init_pci' at ../util/vfio-helpers.c:326:18,
inlined from 'qemu_vfio_open_pci' at ../util/vfio-helpers.c:517:9:
/usr/include/bits/unistd.h:119:10: error: argument 2 is null but the corresponding size argument 3 value is 4095 [-Werror=nonnull]
119 | return __glibc_fortify (readlink, __len, sizeof (char),
| ^~~~~~~~~~~~~~~
This error implies the allocated buffer can be NULL. Use
g_file_read_link(), which allocates buffer automatically to avoid the
error.
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
util/vfio-helpers.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c
index 2d8af38f88..e482ab22e2 100644
--- a/util/vfio-helpers.c
+++ b/util/vfio-helpers.c
@@ -106,15 +106,17 @@ struct QEMUVFIOState {
*/
static char *sysfs_find_group_file(const char *device, Error **errp)
{
+ g_autoptr(GError) gerr;
char *sysfs_link;
char *sysfs_group;
char *p;
char *path = NULL;
sysfs_link = g_strdup_printf("/sys/bus/pci/devices/%s/iommu_group", device);
- sysfs_group = g_malloc0(PATH_MAX);
- if (readlink(sysfs_link, sysfs_group, PATH_MAX - 1) == -1) {
- error_setg_errno(errp, errno, "Failed to find iommu group sysfs path");
+ sysfs_group = g_file_read_link(sysfs_link, &gerr);
+ if (gerr) {
+ error_setg(errp, "Failed to find iommu group sysfs path: %s",
+ gerr->message);
goto out;
}
p = strrchr(sysfs_group, '/');
--
2.40.1
Hello Akihiko, On 5/22/23 13:49, Akihiko Odaki wrote: > When _FORTIFY_SOURCE=2, glibc version is 2.35, and GCC version is > 12.1.0, the compiler complains as follows: > > In file included from /usr/include/features.h:490, > from /usr/include/bits/libc-header-start.h:33, > from /usr/include/stdint.h:26, > from /usr/lib/gcc/aarch64-unknown-linux-gnu/12.1.0/include/stdint.h:9, > from /home/alarm/q/var/qemu/include/qemu/osdep.h:94, > from ../util/vfio-helpers.c:13: > In function 'readlink', > inlined from 'sysfs_find_group_file' at ../util/vfio-helpers.c:116:9, > inlined from 'qemu_vfio_init_pci' at ../util/vfio-helpers.c:326:18, > inlined from 'qemu_vfio_open_pci' at ../util/vfio-helpers.c:517:9: > /usr/include/bits/unistd.h:119:10: error: argument 2 is null but the corresponding size argument 3 value is 4095 [-Werror=nonnull] > 119 | return __glibc_fortify (readlink, __len, sizeof (char), > | ^~~~~~~~~~~~~~~ > > This error implies the allocated buffer can be NULL. Use > g_file_read_link(), which allocates buffer automatically to avoid the > error. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> There are a few other VFIO routines computing the IOMMU group from sysfs : vfio_ap_get_group() vfio_ccw_get_group() vfio_realize() May be there is a possible common routine to introduce. Anyhow, this is beyond the scope of this fix Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > util/vfio-helpers.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c > index 2d8af38f88..e482ab22e2 100644 > --- a/util/vfio-helpers.c > +++ b/util/vfio-helpers.c > @@ -106,15 +106,17 @@ struct QEMUVFIOState { > */ > static char *sysfs_find_group_file(const char *device, Error **errp) > { > + g_autoptr(GError) gerr; > char *sysfs_link; > char *sysfs_group; > char *p; > char *path = NULL; > > sysfs_link = g_strdup_printf("/sys/bus/pci/devices/%s/iommu_group", device); > - sysfs_group = g_malloc0(PATH_MAX); > - if (readlink(sysfs_link, sysfs_group, PATH_MAX - 1) == -1) { > - error_setg_errno(errp, errno, "Failed to find iommu group sysfs path"); > + sysfs_group = g_file_read_link(sysfs_link, &gerr); > + if (gerr) { > + error_setg(errp, "Failed to find iommu group sysfs path: %s", > + gerr->message); > goto out; > } > p = strrchr(sysfs_group, '/');
On 22/5/23 13:49, Akihiko Odaki wrote: > When _FORTIFY_SOURCE=2, glibc version is 2.35, and GCC version is > 12.1.0, the compiler complains as follows: > > In file included from /usr/include/features.h:490, > from /usr/include/bits/libc-header-start.h:33, > from /usr/include/stdint.h:26, > from /usr/lib/gcc/aarch64-unknown-linux-gnu/12.1.0/include/stdint.h:9, > from /home/alarm/q/var/qemu/include/qemu/osdep.h:94, > from ../util/vfio-helpers.c:13: > In function 'readlink', > inlined from 'sysfs_find_group_file' at ../util/vfio-helpers.c:116:9, > inlined from 'qemu_vfio_init_pci' at ../util/vfio-helpers.c:326:18, > inlined from 'qemu_vfio_open_pci' at ../util/vfio-helpers.c:517:9: > /usr/include/bits/unistd.h:119:10: error: argument 2 is null but the corresponding size argument 3 value is 4095 [-Werror=nonnull] > 119 | return __glibc_fortify (readlink, __len, sizeof (char), > | ^~~~~~~~~~~~~~~ > > This error implies the allocated buffer can be NULL. Use > g_file_read_link(), which allocates buffer automatically to avoid the > error. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > --- > util/vfio-helpers.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c > index 2d8af38f88..e482ab22e2 100644 > --- a/util/vfio-helpers.c > +++ b/util/vfio-helpers.c > @@ -106,15 +106,17 @@ struct QEMUVFIOState { > */ > static char *sysfs_find_group_file(const char *device, Error **errp) > { > + g_autoptr(GError) gerr; Shouldn't this also be NULL-initialized (other picky compilers)? Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > char *sysfs_link; > char *sysfs_group; > char *p; > char *path = NULL; > > sysfs_link = g_strdup_printf("/sys/bus/pci/devices/%s/iommu_group", device); > - sysfs_group = g_malloc0(PATH_MAX); > - if (readlink(sysfs_link, sysfs_group, PATH_MAX - 1) == -1) { > - error_setg_errno(errp, errno, "Failed to find iommu group sysfs path"); > + sysfs_group = g_file_read_link(sysfs_link, &gerr); > + if (gerr) { > + error_setg(errp, "Failed to find iommu group sysfs path: %s", > + gerr->message); > goto out; > } > p = strrchr(sysfs_group, '/');
On Mon, May 22, 2023 at 02:38:44PM +0200, Philippe Mathieu-Daudé wrote: > On 22/5/23 13:49, Akihiko Odaki wrote: > > When _FORTIFY_SOURCE=2, glibc version is 2.35, and GCC version is > > 12.1.0, the compiler complains as follows: > > > > In file included from /usr/include/features.h:490, > > from /usr/include/bits/libc-header-start.h:33, > > from /usr/include/stdint.h:26, > > from /usr/lib/gcc/aarch64-unknown-linux-gnu/12.1.0/include/stdint.h:9, > > from /home/alarm/q/var/qemu/include/qemu/osdep.h:94, > > from ../util/vfio-helpers.c:13: > > In function 'readlink', > > inlined from 'sysfs_find_group_file' at ../util/vfio-helpers.c:116:9, > > inlined from 'qemu_vfio_init_pci' at ../util/vfio-helpers.c:326:18, > > inlined from 'qemu_vfio_open_pci' at ../util/vfio-helpers.c:517:9: > > /usr/include/bits/unistd.h:119:10: error: argument 2 is null but the corresponding size argument 3 value is 4095 [-Werror=nonnull] > > 119 | return __glibc_fortify (readlink, __len, sizeof (char), > > | ^~~~~~~~~~~~~~~ > > > > This error implies the allocated buffer can be NULL. Use > > g_file_read_link(), which allocates buffer automatically to avoid the > > error. > > > > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com> > > --- > > util/vfio-helpers.c | 8 +++++--- > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/util/vfio-helpers.c b/util/vfio-helpers.c > > index 2d8af38f88..e482ab22e2 100644 > > --- a/util/vfio-helpers.c > > +++ b/util/vfio-helpers.c > > @@ -106,15 +106,17 @@ struct QEMUVFIOState { > > */ > > static char *sysfs_find_group_file(const char *device, Error **errp) > > { > > + g_autoptr(GError) gerr; > > Shouldn't this also be NULL-initialized (other picky compilers)? Yes, *all* use of g_auto* must have an initializer at time of declaration. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
© 2016 - 2024 Red Hat, Inc.