[libdrm] xf86drm: fix null termination of string buffer

Commit Message

[e-mail] Dec. 13, 2016, 11:18 a.m. UTC

From: Taro Yamada <[e-mail]>

The string written to the buffer by read() is not null-terminated,
but currently drmParsePciBusInfo() places null character only at the end of the buffer, not at the end of the
string.
As a result, the string passed to sscanf() contains an uninitialized value.

This patch changes to places null character at the end of the string.

Signed-off-by: Taro Yamada <[e-mail]>
---
 xf86drm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Emil Velikov Dec. 14, 2016, 3:23 a.m. UTC | #1

Hi Taro Yamada,

On 13 December 2016 at 11:18,  <[e-mail]> wrote:
> From: Taro Yamada <[e-mail]>
>
> The string written to the buffer by read() is not null-terminated,
> but currently drmParsePciBusInfo() places null character only at the end of the buffer, not at the end of the
> string.
> As a result, the string passed to sscanf() contains an uninitialized value.
>
> This patch changes to places null character at the end of the string.
>
Did you see this causing issues in practise ?

We use a combination of strstr to strip any unwanted starting data,
and sscanf which [should] trim any trailing garbage.
That aside, your patch provides an extra bit of robustness which is
always nice ^_^

Reviewed-by: Emil Velikov <[e-mail]>

Barring any objections I'll commit this in a few days.

Thanks
Emil

[e-mail] Dec. 14, 2016, 7:22 a.m. UTC | #2

On 12/14/2016 12:23 PM, Emil Velikov wrote:
> Did you see this causing issues in practise ?
>
> We use a combination of strstr to strip any unwanted starting data,
> and sscanf which [should] trim any trailing garbage.
> That aside, your patch provides an extra bit of robustness which is
> always nice ^_^
No, I've not encountering any problem in practice.
I completely agree with you.

> Barring any objections I'll commit this in a few days.
Thank you for your prompt reply!

Patch

diff --git a/xf86drm.c b/xf86drm.c
index b5eeeb0..a59cfd0 100644
--- a/xf86drm.c
+++ b/xf86drm.c
@@ -2925,11 +2925,11 @@  static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info)
     if (fd < 0)
         return -errno;
 
-    ret = read(fd, data, sizeof(data));
-    data[sizeof(data)-1] = '\0';
+    ret = read(fd, data, sizeof(data)-1);
     close(fd);
     if (ret < 0)
         return -errno;
+    data[ret] = '\0';
 
 #define TAG "PCI_SLOT_NAME="
     str = strstr(data, TAG);