On Tue, 03/14 17:09, Stefan Hajnoczi wrote: > The following pattern is unsafe: > > char buf[32]; > ret = read(fd, buf, sizeof(buf)); > ... > buf[ret] = 0; > > If read(2) returns 32 then a byte beyond the end of the buffer is > zeroed. > > In practice this buffer overflow does not occur because the sysfs > max_segments file only contains an unsigned short + '\n'. The string is > always shorter than 32 bytes. > > Regardless, avoid this pattern because static analysis tools might > complain and it could lead to real buffer overflows if copy-pasted > elsewhere in the codebase.
Yes that's a good point, thanks! Fam > > Signed-off-by: Stefan Hajnoczi <[email protected]> > --- > block/file-posix.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/block/file-posix.c b/block/file-posix.c > index c4c0663..ac6bd9f 100644 > --- a/block/file-posix.c > +++ b/block/file-posix.c > @@ -686,7 +686,7 @@ static int hdev_get_max_segments(const struct stat *st) > goto out; > } > do { > - ret = read(fd, buf, sizeof(buf)); > + ret = read(fd, buf, sizeof(buf) - 1); > } while (ret == -1 && errno == EINTR); > if (ret < 0) { > ret = -errno; > -- > 2.9.3 >
