nul-terminate readlink result
PATCH 1/1 nul-terminate readlink result
readlink by itself does not guarantee that its result is properly nul-terminated. Setting the last byte of the buffer to '\0' fixes this issue.
--- libkms/linux.c | 1 + 1 file changed, 1 insertion(+)
libkms/linux.c +1 -0
@@ -82,6 +82,7 @@ linux_name_from_sysfs(int fd, char **out)if (readlink (path, link,PATH_SIZE ) <0 )return -EINVAL ; + link[PATH_SIZE ] ='\0' ;/* link looks something like this: ../../../bus/pci/drivers/intel */ slash_name =strrchr (link,'/' );
Signature · 6 lines
2.4.5 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Review
Quoted 3 lines · Tobias Stoeckmann
(quoted diff) +1
-0if (readlink(path, link, PATH_SIZE) < 0) return -EINVAL; + link[PATH_SIZE] = '\0';
Sorry, this patch is wrong. It has to be terminated at the correct position. Please see this updated version:
From a1840916dd370d5c84ebf23df91f1c13d564d212 Mon Sep 17 00:00:00 2001
From: Tobias Stoeckmann <[email protected]>
Date: Sun, 12 Jul 2015 12:10:59 +0200
Subject: [PATCH] nul-terminate readlink result
readlink by itself does not terminate its result. The caller has to terminate the string at the proper position.
--- libkms/linux.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
libkms/linux.c +3 -1
@@ -55,6 +55,7 @@ linux_name_from_sysfs(int fd, char **out)unsigned maj, min;char * slash_name;int ret; +ssize_t len;/* * Inside the sysfs directory for the device there is a symlink
@@ -80,8 +81,9 @@ linux_name_from_sysfs(int fd, char **out)snprintf (path,PATH_SIZE ,"/sys/dev/char/%d:%d/device/driver" , maj, min); -if (readlink (path, link,PATH_SIZE ) <0 ) +if ((len =readlink (path, link,PATH_SIZE )) <0 )return -EINVAL ; + link[len] ='\0' ;/* link looks something like this: ../../../bus/pci/drivers/intel */ slash_name =strrchr (link,'/' );
Signature · 6 lines
2.4.5 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Hi Tobias,
Quoted 8 lines · Tobias Stoeckmann
Quoted 3 lines · Tobias Stoeckmann
if (
readlink(path, link, PATH_SIZE)< 0)
return-EINVAL;
+ link[PATH_SIZE] = '\0';Sorry, this patch is wrong. It has to be terminated at the correct position. Please see this updated version:
Thanks for the update, there is a pick I'm afraid:
Quoted 25 lines
From
a1840916dd370d5c84ebf23df91f1c13d564d212Mon Sep 17 00:00:00 2001
From: Tobias Stoeckmann <[email protected]>
Date: Sun, 12 Jul 2015 12:10:59 +0200
Subject: [PATCH] nul-terminate readlink resultreadlink by itself does not terminate its result. The caller has to terminate the string at the proper position.
--- libkms/linux.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)libkms/linux.c +1
-0@@ -55,6 +55,7 @@ linux_name_from_sysfs(int fd, char **out)unsigned maj, min;char * slash_name;int ret; +ssize_t len;/* * Inside the sysfs directory for the device there is a symlink
libkms/linux.c linux_name_from_sysfs(int fd, char **out) · line 86
Quoted 7 lines
libkms/linux.c +2
-1@@ -80,8 +81,9 @@ linux_name_from_sysfs(int fd, char **out)snprintf (path,PATH_SIZE ,"/sys/dev/char/%d:%d/device/driver" , maj, min); -if (readlink (path, link,PATH_SIZE ) <0 ) +if ((len =readlink (path, link,PATH_SIZE )) <0 )return -EINVAL ; + link[len] ='\0' ;
The man page does not say it -1 + errno is returned when the buffer is too small.
There is ENAMETOOLONG which reads "A pathname, or a component of a pathname, was too long.". At the same time the example given explicitly checks for len >= PATH_SIZE.
It uses lstat + malloc over a static storage. Not sure how much this matters here though.
Thanks
Emil
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel