On Fri, Jan 22, 2016 at 04:48:05PM +0200, Ville Syrjälä wrote:

On Fri, Jan 22, 2016 at 12:51:23PM +0000, Damien Lespiau wrote:

...

We are reading at most sizeof(data) bytes, but then data may not contain a terminating '\0', at least in theory, so strstr() may overflow the stack allocated array.

Make sure that data always contains at least one '\0'.

Signed-off-by: Damien Lespiau damien.lespiau@intel.com

xf86drm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xf86drm.c b/xf86drm.c index 7e28b4f..5f587d9 100644 --- a/xf86drm.c +++ b/xf86drm.c @@ -2863,7 +2863,7 @@ static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info) { #ifdef __linux__ char path[PATH_MAX + 1];

• char data[128];

• char data[128 + 1]; char *str; int domain, bus, dev, func; int fd, ret;

@@ -2874,6 +2874,7 @@ static int drmParsePciBusInfo(int maj, int min, drmPciBusInfoPtr info) return -errno;

 ret = read(fd, data, sizeof(data));

• data[128] = '\0';

Slightly more paranoid would be something along the lines of if (ret >= 0) data[ret] = '\0';

But this should be good enough I think so Reviewed-by: Ville Syrjälä ville.syrjala@linux.intel.com

Thanks for the review, pushed!

The other thing I spotted while looking at the code is the fact that it doesn't check the snprint() return value. But I guess PATH_MAX is big enough that even if you somehow make maj and min INT_MIN it'll still fit.

Right, doesn't seem we can overflow path[].