Bug 260343 - NFS client does not respect RLIMIT_FSIZE
Summary: NFS client does not respect RLIMIT_FSIZE
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 13.0-RELEASE
Hardware: Any Any
: --- Affects Only Me
Assignee: Rick Macklem
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-12-11 17:45 UTC by Alan Somers
Modified: 2021-12-27 01:01 UTC (History)
1 user (show)

See Also:
rmacklem: mfc-stable13+
rmacklem: mfc-stable12-


Attachments
add rlimit filesize check to nfs_allocate (1.30 KB, patch)
2021-12-13 22:10 UTC, Rick Macklem
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Somers freebsd_committer freebsd_triage 2021-12-11 17:45:52 UTC
RLIMIT_FIZE limits the maximum size of file that a process can create.  It should be checked during any syscall that can increase the size of a file.  However, it is ignored by the NFS client's VOP_ALLOCATE implementation.

nfsrpc_allocate should probably have the same check for vn_rlimit_fsize() that nfs_copy_file_range and ncl_write do.
Comment 1 Rick Macklem freebsd_committer freebsd_triage 2021-12-12 02:08:30 UTC
Yep. I suppose vn_rlimit_fsize() should be called in
nfs_allocate().

It should also be added to vop_stdallocate() to be consistent
w.r.t. its use.

(Note that VOP_SETATTR() of size does not check it and never
 has, for UFS, NFS,... It can grow the file size, although it
 does not allocate space.)

Maybe use of RLIMIT_FSIZE needs to be discussed on a mailing list,
to agree on what VOP calls should check it?
Comment 2 Alan Somers freebsd_committer freebsd_triage 2021-12-12 02:12:36 UTC
Isn't vop_stdallocate covered by the call to VOP_WRITE, which checks RLIMIT_FSIZE?  Or are you concerned that RLIMIT_FSIZE needs to be checked before vop_stdallocate's loop begins?

Bug 164793 describes RLIMIT_FSIZE with truncate.  It's pretty clearly a POSIX violation that we don't check it.  I guess nobody cares, though.  RLIMIT_FSIZE isn't all that important.
Comment 3 Rick Macklem freebsd_committer freebsd_triage 2021-12-12 02:20:21 UTC
Yes, I just noticed that vop_stdallocate() uses VOP_WRITE(),
so it does get checked.

I'll add it to nfs_allocate().

(I suspect long ago that VOP_SETATTR() of size was exempt
 because it does not allocate storage and the limit was
 really meant to limit storage usage and not size?
 I could be wrong, but I think rlimit was a CSRG BSD thing
 way back in the 1980s. POSIX probably picked up on it.)
Comment 4 Rick Macklem freebsd_committer freebsd_triage 2021-12-13 22:10:45 UTC
Created attachment 230091 [details]
add rlimit filesize check to nfs_allocate

This patch adds a check for filesize rlimit to
nfs_allocate().
Comment 5 Alan Somers freebsd_committer freebsd_triage 2021-12-13 22:14:09 UTC
Comment on attachment 230091 [details]
add rlimit filesize check to nfs_allocate

LGTM.
Comment 6 commit-hook freebsd_committer freebsd_triage 2021-12-13 23:36:50 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=fe04c91184e9e82609a657c4e6e70e213ed3a859

commit fe04c91184e9e82609a657c4e6e70e213ed3a859
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-12-13 23:32:19 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-12-13 23:32:19 +0000

    nfscl: add a filesize limit check to nfs_allocate()

    As reported in PR#260343, nfs_allocate() did not check
    the filesize rlimit. This patch adds that check.

    PR:     260343
    Reviewed by:    asomers
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D33422

 sys/fs/nfsclient/nfs_clvnops.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
Comment 7 Rick Macklem freebsd_committer freebsd_triage 2021-12-13 23:39:01 UTC
Patch has been committed and will be MFC'd.
Comment 8 commit-hook freebsd_committer freebsd_triage 2021-12-27 00:59:01 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=3c3b641a610cf57577c26850edb9a1de2c024f96

commit 3c3b641a610cf57577c26850edb9a1de2c024f96
Author:     Rick Macklem <rmacklem@FreeBSD.org>
AuthorDate: 2021-12-13 23:32:19 +0000
Commit:     Rick Macklem <rmacklem@FreeBSD.org>
CommitDate: 2021-12-27 00:55:09 +0000

    nfscl: add a filesize limit check to nfs_allocate()

    As reported in PR#260343, nfs_allocate() did not check
    the filesize rlimit. This patch adds that check.

    PR:     260343

    (cherry picked from commit fe04c91184e9e82609a657c4e6e70e213ed3a859)

 sys/fs/nfsclient/nfs_clvnops.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)
Comment 9 Rick Macklem freebsd_committer freebsd_triage 2021-12-27 01:01:11 UTC
Patch has been committed and MFC'd.