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[].