On Wed, Nov 22, 2023 at 4:34 PM Christian Brauner <brauner@xxxxxxxxxx> wrote:
>
> > - ret = import_iovec(ITER_SOURCE, vec, vlen, ARRAY_SIZE(iovstack), &iov, &iter);
> > - if (ret >= 0) {
> > - file_start_write(file);
> > - ret = do_iter_write(file, &iter, pos, flags);
> > - file_end_write(file);
> > - kfree(iov);
> > - }
> > + if (!(file->f_mode & FMODE_WRITE))
> > + return -EBADF;
> > + if (!(file->f_mode & FMODE_CAN_WRITE))
> > + return -EINVAL;
> > +
> > + ret = import_iovec(ITER_SOURCE, vec, vlen, ARRAY_SIZE(iovstack), &iov,
> > + &iter);
> > + if (ret < 0)
> > + return ret;
> > +
> > + tot_len = iov_iter_count(&iter);
> > + if (!tot_len)
> > + goto out;
>
> Fwiw, the logic is slightly changed here. This now relies on
> import_iovec() >= 0 then iov_iter_count() >= 0.
>
> If that's ever changed and iov_iter_count() can return an error even
> though import_iovec() succeeded we'll be returning the number of
> imported bytes even though nothing was written and also generate a
> fsnotify event because ret still points to the number of imported bytes.
>
> The way it was written before it didn't matter because this was hidden
> in a function call that returned 0 and initialized ret again.
Good catch!
> Anyway, I
> can just massage that in-tree if that's worth it. Nothing to do for you.
Thank you!
Amir.
