| Message ID | 20180730190146.GA8535@localhost (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Verify memory sizes on 32 bit systems. | expand |
On Mon, Jul 30, 2018 at 12:02 PM Tobias Stoeckmann <tobias@stoeckmann.org> wrote: > > Large file system support is activated by default, which means that > on 32 bit systems, off_t is 64 bit in size. > > Using st.st_size or any other 64 bit variable with mmap can lead to > integer truncation and therefore insufficient memory mapping. > > Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> > --- > libkmod/libkmod-file.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/libkmod/libkmod-file.c b/libkmod/libkmod-file.c > index 5eeba6a..86f34c6 100644 > --- a/libkmod/libkmod-file.c > +++ b/libkmod/libkmod-file.c > @@ -255,6 +255,8 @@ static int load_reg(struct kmod_file *file) > return -errno; > > file->size = st.st_size; > + if ((uintmax_t)st.st_size > (uintmax_t)SIZE_MAX) why casting? Lucas De Marchi > + return -EFBIG; > file->memory = mmap(NULL, file->size, PROT_READ, MAP_PRIVATE, > file->fd, 0); > if (file->memory == MAP_FAILED) > -- > 2.13.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-modules" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 03, 2018 at 02:07:34PM -0700, Lucas De Marchi wrote: > > + if ((uintmax_t)st.st_size > (uintmax_t)SIZE_MAX) > > why casting? Without casting, a warning could be triggered on 64 bit systems, because on these systems a signed 64 bit int (off_t) is compared with an unsigned 64 bit int (size_t) which could never be larger. Casting it to uintmax_t, which is at least 64 bits in size, silences this error. No technical reason beyond that. Tobias -- To unsubscribe from this list: send the line "unsubscribe linux-modules" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Aug 3, 2018 at 2:10 PM Tobias Stoeckmann <tobias@stoeckmann.org> wrote: > > On Fri, Aug 03, 2018 at 02:07:34PM -0700, Lucas De Marchi wrote: > > > + if ((uintmax_t)st.st_size > (uintmax_t)SIZE_MAX) > > > > why casting? > > Without casting, a warning could be triggered on 64 bit systems, > because on these systems a signed 64 bit int (off_t) is compared > with an unsigned 64 bit int (size_t) which could never be larger. > > Casting it to uintmax_t, which is at least 64 bits in size, > silences this error. so you are casting the *sign* away, ok. But casting only st.st_size should suffice. Lucas De Marchi > > No technical reason beyond that. > > > Tobias
On Fri, Aug 03, 2018 at 02:33:51PM -0700, Lucas De Marchi wrote: > so you are casting the *sign* away, ok. But casting only st.st_size > should suffice. I've just tried with some examples and I couldn't trigger the warning which I have seen in other situations like this, i.e. "always false due to limited range of data type". If I cast the off_t to size_t, the compiler knows that it is not possible that it could ever be larger than SIZE_MAX, optimizing the block away. So I took uintmax_t. But as long as there's no annoying warning, I am fine with just casting st_size. Tobias -- To unsubscribe from this list: send the line "unsubscribe linux-modules" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/libkmod/libkmod-file.c b/libkmod/libkmod-file.c index 5eeba6a..86f34c6 100644 --- a/libkmod/libkmod-file.c +++ b/libkmod/libkmod-file.c @@ -255,6 +255,8 @@ static int load_reg(struct kmod_file *file) return -errno; file->size = st.st_size; + if ((uintmax_t)st.st_size > (uintmax_t)SIZE_MAX) + return -EFBIG; file->memory = mmap(NULL, file->size, PROT_READ, MAP_PRIVATE, file->fd, 0); if (file->memory == MAP_FAILED)
Large file system support is activated by default, which means that on 32 bit systems, off_t is 64 bit in size. Using st.st_size or any other 64 bit variable with mmap can lead to integer truncation and therefore insufficient memory mapping. Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org> --- libkmod/libkmod-file.c | 2 ++ 1 file changed, 2 insertions(+)