On Thu, Oct 18, 2018 at 01:41:56AM +0100, Al Viro wrote:
> On Wed, Oct 17, 2018 at 03:44:43PM -0700, Darrick J. Wong wrote:
> > +static int generic_access_check_limits(struct file *file, loff_t pos,
> > + loff_t *count)
> > +{
> > + struct inode *inode = file->f_mapping->host;
> > +
> > + /* Don't exceed the LFS limits. */
> > + if (unlikely(pos + *count > MAX_NON_LFS &&
> > + !(file->f_flags & O_LARGEFILE))) {
> > + if (pos >= MAX_NON_LFS)
> > + return -EFBIG;
> > + *count = min(*count, (loff_t)MAX_NON_LFS - pos);
>
> Can that can be different from MAX_NON_LFS - pos?
>
> > + }
> > +
> > + /*
> > + * Don't operate on ranges the page cache doesn't support.
> > + *
> > + * If we have written data it becomes a short write. If we have
> > + * exceeded without writing data we send a signal and return EFBIG.
> > + * Linus frestrict idea will clean these up nicely..
> > + */
> > + if (unlikely(pos >= inode->i_sb->s_maxbytes))
> > + return -EFBIG;
> > +
> > + *count = min(*count, inode->i_sb->s_maxbytes - pos);
> > + return 0;
> > +}
>
> Anyway, I would rather do this here:
>
> struct inode *inode = file->f_mapping->host;
> loff_t max_size = inode->i_sb->s_maxbytes;
>
> if (!(file->f_flags & O_LARGEFILE))
> max_size = MAX_NON_LFS;
>
> if (unlikely(pos >= max_size))
> return -EFBIG;
> *count = min(*count, max_size - pos);
> return 0;
Sounds much better to me. :)
--D
