[libdrm,2/3] xf86drm: add buffer size safety to sprintf()

Commit Message

Eric Engestrom March 26, 2018, 10:26 a.m. UTC

Signed-off-by: Eric Engestrom <[e-mail]>
---
 xf86drm.c     | 6 +++---
 xf86drmMode.c | 6 ++++--
 2 files changed, 7 insertions(+), 5 deletions(-)

Comments

Emil Velikov March 26, 2018, 2:02 p.m. UTC | #1

On 26 March 2018 at 11:26, Eric Engestrom <[e-mail]> wrote:
> Signed-off-by: Eric Engestrom <[e-mail]>
> ---
>  xf86drm.c     | 6 +++---
>  xf86drmMode.c | 6 ++++--
>  2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/xf86drm.c b/xf86drm.c
> index b6e5d8cc1bb50ffe75a2..5701952ae83634b47628 100644
> --- a/xf86drm.c
> +++ b/xf86drm.c
> @@ -349,7 +349,7 @@ static int drmOpenDevice(dev_t dev, int minor, int type)
>          return -EINVAL;
>      };
>
> -    sprintf(buf, dev_name, DRM_DIR_NAME, minor);
> +    snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, minor);
The patch feels a big meh, for two reasons:
 - buffer contents are controlled and will never overflow
 - s{n,}printf return value is not checked

If there's a particular tool that should be silenced up sure, let's
land it. Otherwise I might as well list my 'grand master plan', we can
start deleting all this code ;-)

What do you think?
Emil

Patch

diff --git a/xf86drm.c b/xf86drm.c
index b6e5d8cc1bb50ffe75a2..5701952ae83634b47628 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -349,7 +349,7 @@  static int drmOpenDevice(dev_t dev, int minor, int type)
         return -EINVAL;
     };
 
-    sprintf(buf, dev_name, DRM_DIR_NAME, minor);
+    snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, minor);
     drmMsg("drmOpenDevice: node name is %s\n", buf);
 
     if (drm_server_info && drm_server_info->get_perms) {
@@ -473,7 +473,7 @@  static int drmOpenMinor(int minor, int create, int type)
         return -EINVAL;
     };
 
-    sprintf(buf, dev_name, DRM_DIR_NAME, minor);
+    snprintf(buf, sizeof(buf), dev_name, DRM_DIR_NAME, minor);
     if ((fd = open(buf, O_RDWR, 0)) >= 0)
         return fd;
     return -errno;
@@ -681,7 +681,7 @@  static int drmOpenByName(const char *name, int type)
         char *driver, *pt, *devstring;
         int  retcode;
 
-        sprintf(proc_name, "/proc/dri/%d/name", i);
+        snprintf(proc_name, sizeof(proc_name), "/proc/dri/%d/name", i);
         if ((fd = open(proc_name, 0, 0)) >= 0) {
             retcode = read(fd, buf, sizeof(buf)-1);
             close(fd);
diff --git a/xf86drmMode.c b/xf86drmMode.c
index 9a15b5e78dda9a4bb7e4..d482ef79c0dc35d639d0 100644
--- a/xf86drmMode.c
+++ b/xf86drmMode.c
@@ -753,7 +753,8 @@  int drmCheckModesettingSupported(const char *busid)
 	if (ret != 4)
 		return -EINVAL;
 
-	sprintf(pci_dev_dir, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/drm",
+	snprintf(pci_dev_dir, sizeof(pci_dev_dir),
+		"/sys/bus/pci/devices/%04x:%02x:%02x.%d/drm",
 		domain, bus, dev, func);
 
 	sysdir = opendir(pci_dev_dir);
@@ -772,7 +773,8 @@  int drmCheckModesettingSupported(const char *busid)
 			return 0;
 	}
 
-	sprintf(pci_dev_dir, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/",
+	snprintf(pci_dev_dir, sizeof(pci_dev_dir),
+		"/sys/bus/pci/devices/%04x:%02x:%02x.%d/",
 		domain, bus, dev, func);
 
 	sysdir = opendir(pci_dev_dir);