| Message ID | 20240906033202.1252195-1-sunjunchao2870@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vfs: return -EOVERFLOW in generic_remap_checks() when overflow check fails | expand |
On Fri 06-09-24 11:32:02, Julian Sun wrote: > Keep it consistent with the handling of the same check within > generic_copy_file_checks(). > Also, returning -EOVERFLOW in this case is more appropriate. > > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com> Well, you were already changing this condition here [1] so maybe just update the errno in that patch as well? No need to generate unnecessary patch conflicts... [1] https://lore.kernel.org/all/20240905121545.ma6zdnswn5s72byb@quack3 Honza > --- > fs/remap_range.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/remap_range.c b/fs/remap_range.c > index 28246dfc8485..97171f2191aa 100644 > --- a/fs/remap_range.c > +++ b/fs/remap_range.c > @@ -46,7 +46,7 @@ static int generic_remap_checks(struct file *file_in, loff_t pos_in, > > /* Ensure offsets don't wrap. */ > if (pos_in + count < pos_in || pos_out + count < pos_out) > - return -EINVAL; > + return -EOVERFLOW; > > size_in = i_size_read(inode_in); > size_out = i_size_read(inode_out); > -- > 2.39.2 >
On Fri, 2024-09-06 at 12:29 +0200, Jan Kara wrote: Sure, I will include this patch in the patch set for the next version. But I think it maybe deserves a separate patch, rather than being integrated into the original patch? > On Fri 06-09-24 11:32:02, Julian Sun wrote: > > Keep it consistent with the handling of the same check within > > generic_copy_file_checks(). > > Also, returning -EOVERFLOW in this case is more appropriate. > > > > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com> > > Well, you were already changing this condition here [1] so maybe just > update the errno in that patch as well? No need to generate unnecessary > patch conflicts... > > [1] https://lore.kernel.org/all/20240905121545.ma6zdnswn5s72byb@quack3 > > Honza > > > --- > > fs/remap_range.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/fs/remap_range.c b/fs/remap_range.c > > index 28246dfc8485..97171f2191aa 100644 > > --- a/fs/remap_range.c > > +++ b/fs/remap_range.c > > @@ -46,7 +46,7 @@ static int generic_remap_checks(struct file *file_in, loff_t pos_in, > > > > /* Ensure offsets don't wrap. */ > > if (pos_in + count < pos_in || pos_out + count < pos_out) > > - return -EINVAL; > > + return -EOVERFLOW; > > > > size_in = i_size_read(inode_in); > > size_out = i_size_read(inode_out); > > -- > > 2.39.2 > > Thanks,
On Fri 06-09-24 19:12:08, Julian Sun wrote: > On Fri, 2024-09-06 at 12:29 +0200, Jan Kara wrote: > > Sure, I will include this patch in the patch set for the next version. > But I think it maybe deserves a separate patch, rather than being > integrated into the original patch? Yes, probably a separate patch makes sense. Honza > > > On Fri 06-09-24 11:32:02, Julian Sun wrote: > > > Keep it consistent with the handling of the same check within > > > generic_copy_file_checks(). > > > Also, returning -EOVERFLOW in this case is more appropriate. > > > > > > Signed-off-by: Julian Sun <sunjunchao2870@gmail.com> > > > > Well, you were already changing this condition here [1] so maybe just > > update the errno in that patch as well? No need to generate unnecessary > > patch conflicts... > > > > [1] https://lore.kernel.org/all/20240905121545.ma6zdnswn5s72byb@quack3 > > > > Honza > > > > > --- > > > fs/remap_range.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/fs/remap_range.c b/fs/remap_range.c > > > index 28246dfc8485..97171f2191aa 100644 > > > --- a/fs/remap_range.c > > > +++ b/fs/remap_range.c > > > @@ -46,7 +46,7 @@ static int generic_remap_checks(struct file *file_in, loff_t pos_in, > > > > > > /* Ensure offsets don't wrap. */ > > > if (pos_in + count < pos_in || pos_out + count < pos_out) > > > - return -EINVAL; > > > + return -EOVERFLOW; > > > > > > size_in = i_size_read(inode_in); > > > size_out = i_size_read(inode_out); > > > -- > > > 2.39.2 > > > > > Thanks, > -- > Julian Sun <sunjunchao2870@gmail.com>
diff --git a/fs/remap_range.c b/fs/remap_range.c index 28246dfc8485..97171f2191aa 100644 --- a/fs/remap_range.c +++ b/fs/remap_range.c @@ -46,7 +46,7 @@ static int generic_remap_checks(struct file *file_in, loff_t pos_in, /* Ensure offsets don't wrap. */ if (pos_in + count < pos_in || pos_out + count < pos_out) - return -EINVAL; + return -EOVERFLOW; size_in = i_size_read(inode_in); size_out = i_size_read(inode_out);
Keep it consistent with the handling of the same check within generic_copy_file_checks(). Also, returning -EOVERFLOW in this case is more appropriate. Signed-off-by: Julian Sun <sunjunchao2870@gmail.com> --- fs/remap_range.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)