| Message ID | 20170111204204.7727-2-robert.foss@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Robert Foss schreef op wo 11-01-2017 om 15:41 [-0500]: > buf array may overflow with when writing '\0' if > MAX_LINE_LEN bytes are read during read(). How? char buf[MAX_LINE_LEN + 1]; > Signed-off-by: Robert Foss <robert.foss@collabora.com> > --- > lib/igt_debugfs.c | 8 +++++--- > 1 file changed, 5 insertions(+), 3 deletions(-) > > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c > index d828687a..8b8a627a 100644 > --- a/lib/igt_debugfs.c > +++ b/lib/igt_debugfs.c > @@ -594,13 +594,15 @@ static int read_crc(igt_pipe_crc_t *pipe_crc, > igt_crc_t *out) > read_len = MAX_LINE_LEN; > > igt_set_timeout(5, "CRC reading"); > - bytes_read = read(pipe_crc->crc_fd, &buf, read_len); > + bytes_read = read(pipe_crc->crc_fd, &buf, read_len - 1); > igt_reset_timeout(); > > - if (bytes_read < 0 && errno == EAGAIN) { > + if (bytes_read < 0 && errno == EAGAIN) > igt_assert(pipe_crc->flags & O_NONBLOCK); > + > + if (bytes_read < 0) > bytes_read = 0; > - } > + > buf[bytes_read] = '\0'; > > if (bytes_read && !pipe_crc_init_from_string(pipe_crc, out, > buf))
On 2017-01-12 04:14 AM, Lankhorst, Maarten wrote: > Robert Foss schreef op wo 11-01-2017 om 15:41 [-0500]: >> buf array may overflow with when writing '\0' if >> MAX_LINE_LEN bytes are read during read(). > How? > > char buf[MAX_LINE_LEN + 1]; I actually missed the + 1, but parts of the commit are still relevant though, as the errno at least in theory could be != EAGAIN. So I'd like to keep the below check, to prevent compiler warnings. if (bytes_read < 0) Sounds ok? Rob. > >> Signed-off-by: Robert Foss <robert.foss@collabora.com> >> --- >> lib/igt_debugfs.c | 8 +++++--- >> 1 file changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c >> index d828687a..8b8a627a 100644 >> --- a/lib/igt_debugfs.c >> +++ b/lib/igt_debugfs.c >> @@ -594,13 +594,15 @@ static int read_crc(igt_pipe_crc_t *pipe_crc, >> igt_crc_t *out) >> read_len = MAX_LINE_LEN; >> >> igt_set_timeout(5, "CRC reading"); >> - bytes_read = read(pipe_crc->crc_fd, &buf, read_len); >> + bytes_read = read(pipe_crc->crc_fd, &buf, read_len - 1); >> igt_reset_timeout(); >> >> - if (bytes_read < 0 && errno == EAGAIN) { >> + if (bytes_read < 0 && errno == EAGAIN) >> igt_assert(pipe_crc->flags & O_NONBLOCK); >> + >> + if (bytes_read < 0) >> bytes_read = 0; >> - } >> + >> buf[bytes_read] = '\0'; >> >> if (bytes_read && !pipe_crc_init_from_string(pipe_crc, out, >> buf))
Robert Foss schreef op do 12-01-2017 om 11:30 [-0500]: > > On 2017-01-12 04:14 AM, Lankhorst, Maarten wrote: > > > > Robert Foss schreef op wo 11-01-2017 om 15:41 [-0500]: > > > > > > buf array may overflow with when writing '\0' if > > > MAX_LINE_LEN bytes are read during read(). > > How? > > > > char buf[MAX_LINE_LEN + 1]; > > I actually missed the + 1, but parts of the commit are still > relevant > though, as the errno at least in theory could be != EAGAIN. > > So I'd like to keep the below check, to prevent compiler warnings. > if (bytes_read < 0) > > Sounds ok? Yes. :) > > Rob. > > > > > > > > > > Signed-off-by: Robert Foss <robert.foss@collabora.com> > > > --- > > > lib/igt_debugfs.c | 8 +++++--- > > > 1 file changed, 5 insertions(+), 3 deletions(-) > > > > > > diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c > > > index d828687a..8b8a627a 100644 > > > --- a/lib/igt_debugfs.c > > > +++ b/lib/igt_debugfs.c > > > @@ -594,13 +594,15 @@ static int read_crc(igt_pipe_crc_t > > > *pipe_crc, > > > igt_crc_t *out) > > > read_len = MAX_LINE_LEN; > > > > > > igt_set_timeout(5, "CRC reading"); > > > - bytes_read = read(pipe_crc->crc_fd, &buf, read_len); > > > + bytes_read = read(pipe_crc->crc_fd, &buf, read_len - 1); > > > igt_reset_timeout(); > > > > > > - if (bytes_read < 0 && errno == EAGAIN) { > > > + if (bytes_read < 0 && errno == EAGAIN) > > > igt_assert(pipe_crc->flags & O_NONBLOCK); > > > + > > > + if (bytes_read < 0) > > > bytes_read = 0; > > > - } > > > + > > > buf[bytes_read] = '\0'; > > > > > > if (bytes_read && !pipe_crc_init_from_string(pipe_crc, > > > out, > > > buf))
diff --git a/lib/igt_debugfs.c b/lib/igt_debugfs.c index d828687a..8b8a627a 100644 --- a/lib/igt_debugfs.c +++ b/lib/igt_debugfs.c @@ -594,13 +594,15 @@ static int read_crc(igt_pipe_crc_t *pipe_crc, igt_crc_t *out) read_len = MAX_LINE_LEN; igt_set_timeout(5, "CRC reading"); - bytes_read = read(pipe_crc->crc_fd, &buf, read_len); + bytes_read = read(pipe_crc->crc_fd, &buf, read_len - 1); igt_reset_timeout(); - if (bytes_read < 0 && errno == EAGAIN) { + if (bytes_read < 0 && errno == EAGAIN) igt_assert(pipe_crc->flags & O_NONBLOCK); + + if (bytes_read < 0) bytes_read = 0; - } + buf[bytes_read] = '\0'; if (bytes_read && !pipe_crc_init_from_string(pipe_crc, out, buf))
buf array may overflow with when writing '\0' if MAX_LINE_LEN bytes are read during read(). Signed-off-by: Robert Foss <robert.foss@collabora.com> --- lib/igt_debugfs.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)