Re: [PATCH] fs/nilfs2: Integer overflow in nilfs_ioctl_wrap_copy()
Hi, On Sat, 2013-12-28 at 10:28 +0800, Wenliang Fan wrote: > The local variable 'pos' comes from userspace. If a large number was > passed, there would be an integer overflow in the following line: > pos += n; > > Signed-off-by: Wenliang Fan > --- > fs/nilfs2/ioctl.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c > index b44bdb2..a260a98 100644 > --- a/fs/nilfs2/ioctl.c > +++ b/fs/nilfs2/ioctl.c > @@ -65,6 +65,8 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs, > ret = 0; > total = 0; > pos = argv->v_index; > + if (pos > ULONG_MAX - argv->v_nmembs) > + return -EINVAL; I am slightly confused. Does this patch means that you refused previous one? Or this patch is addition to the previous one? You need to prepare next version of the patch or to prepare patch set from two patches. And it makes sense to provide changelog of your work, I think. Thanks, Vyacheslav Dubeyko. > for (i = 0; i < argv->v_nmembs; i += n) { > n = (argv->v_nmembs - i < maxmembs) ? > argv->v_nmembs - i : maxmembs; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fs/nilfs2: Integer overflow in nilfs_ioctl_wrap_copy()
Hi, On Sat, 2013-12-28 at 10:28 +0800, Wenliang Fan wrote: The local variable 'pos' comes from userspace. If a large number was passed, there would be an integer overflow in the following line: pos += n; Signed-off-by: Wenliang Fan fanwle...@gmail.com --- fs/nilfs2/ioctl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c index b44bdb2..a260a98 100644 --- a/fs/nilfs2/ioctl.c +++ b/fs/nilfs2/ioctl.c @@ -65,6 +65,8 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs, ret = 0; total = 0; pos = argv-v_index; + if (pos ULONG_MAX - argv-v_nmembs) + return -EINVAL; I am slightly confused. Does this patch means that you refused previous one? Or this patch is addition to the previous one? You need to prepare next version of the patch or to prepare patch set from two patches. And it makes sense to provide changelog of your work, I think. Thanks, Vyacheslav Dubeyko. for (i = 0; i argv-v_nmembs; i += n) { n = (argv-v_nmembs - i maxmembs) ? argv-v_nmembs - i : maxmembs; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs/nilfs2: Integer overflow in nilfs_ioctl_wrap_copy()
The local variable 'pos' comes from userspace. If a large number was passed, there would be an integer overflow in the following line: pos += n; Signed-off-by: Wenliang Fan --- fs/nilfs2/ioctl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c index b44bdb2..a260a98 100644 --- a/fs/nilfs2/ioctl.c +++ b/fs/nilfs2/ioctl.c @@ -65,6 +65,8 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs, ret = 0; total = 0; pos = argv->v_index; + if (pos > ULONG_MAX - argv->v_nmembs) + return -EINVAL; for (i = 0; i < argv->v_nmembs; i += n) { n = (argv->v_nmembs - i < maxmembs) ? argv->v_nmembs - i : maxmembs; -- 1.8.5.rc1.28.g7061504 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs/nilfs2: Integer overflow in nilfs_ioctl_wrap_copy()
The local variable 'pos' comes from userspace. If a large number was passed, there would be an integer overflow in the following line: pos += n; Signed-off-by: Wenliang Fan fanwle...@gmail.com --- fs/nilfs2/ioctl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c index b44bdb2..a260a98 100644 --- a/fs/nilfs2/ioctl.c +++ b/fs/nilfs2/ioctl.c @@ -65,6 +65,8 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs, ret = 0; total = 0; pos = argv-v_index; + if (pos ULONG_MAX - argv-v_nmembs) + return -EINVAL; for (i = 0; i argv-v_nmembs; i += n) { n = (argv-v_nmembs - i maxmembs) ? argv-v_nmembs - i : maxmembs; -- 1.8.5.rc1.28.g7061504 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fs/nilfs2: Integer overflow in nilfs_ioctl_wrap_copy()
On Fri, 2013-12-20 at 15:46 +0800, Wenliang Fan wrote: > The local variable 'pos' comes from userspace. If a large number was > passed, there would be an integer overflow in the following line: > pos += n; > What about check before enter into the cycle? If you are talking about possible huge number then you can receive huge number from userspace before entering in the cycle. Thanks, Vyacheslav Dubeyko. > Signed-off-by: Wenliang Fan > --- > fs/nilfs2/ioctl.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c > index b44bdb2..b2bac9a 100644 > --- a/fs/nilfs2/ioctl.c > +++ b/fs/nilfs2/ioctl.c > @@ -90,8 +90,13 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs, > total += nr; > if ((size_t)nr < n) > break; > - if (pos == ppos) > + if (pos == ppos) { > + if (pos > ULONG_MAX - n) { > + ret = -EINVAL; > + break; > + } > pos += n; > + } > } > argv->v_nmembs = total; > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] fs/nilfs2: Integer overflow in nilfs_ioctl_wrap_copy()
On Fri, 2013-12-20 at 15:46 +0800, Wenliang Fan wrote: The local variable 'pos' comes from userspace. If a large number was passed, there would be an integer overflow in the following line: pos += n; What about check before enter into the cycle? If you are talking about possible huge number then you can receive huge number from userspace before entering in the cycle. Thanks, Vyacheslav Dubeyko. Signed-off-by: Wenliang Fan fanwle...@gmail.com --- fs/nilfs2/ioctl.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c index b44bdb2..b2bac9a 100644 --- a/fs/nilfs2/ioctl.c +++ b/fs/nilfs2/ioctl.c @@ -90,8 +90,13 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs, total += nr; if ((size_t)nr n) break; - if (pos == ppos) + if (pos == ppos) { + if (pos ULONG_MAX - n) { + ret = -EINVAL; + break; + } pos += n; + } } argv-v_nmembs = total; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs/nilfs2: Integer overflow in nilfs_ioctl_wrap_copy()
The local variable 'pos' comes from userspace. If a large number was passed, there would be an integer overflow in the following line: pos += n; Signed-off-by: Wenliang Fan --- fs/nilfs2/ioctl.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c index b44bdb2..b2bac9a 100644 --- a/fs/nilfs2/ioctl.c +++ b/fs/nilfs2/ioctl.c @@ -90,8 +90,13 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs, total += nr; if ((size_t)nr < n) break; - if (pos == ppos) + if (pos == ppos) { + if (pos > ULONG_MAX - n) { + ret = -EINVAL; + break; + } pos += n; + } } argv->v_nmembs = total; -- 1.8.5.rc1.28.g7061504 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] fs/nilfs2: Integer overflow in nilfs_ioctl_wrap_copy()
The local variable 'pos' comes from userspace. If a large number was passed, there would be an integer overflow in the following line: pos += n; Signed-off-by: Wenliang Fan fanwle...@gmail.com --- fs/nilfs2/ioctl.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c index b44bdb2..b2bac9a 100644 --- a/fs/nilfs2/ioctl.c +++ b/fs/nilfs2/ioctl.c @@ -90,8 +90,13 @@ static int nilfs_ioctl_wrap_copy(struct the_nilfs *nilfs, total += nr; if ((size_t)nr n) break; - if (pos == ppos) + if (pos == ppos) { + if (pos ULONG_MAX - n) { + ret = -EINVAL; + break; + } pos += n; + } } argv-v_nmembs = total; -- 1.8.5.rc1.28.g7061504 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/