Page MenuHomeFreeBSD
Differential D19702

Ignore F_SETLK_REMOTE requests for sysid 0.
ClosedPublic
Actions

Authored by markj on Mar 25 2019, 4:32 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 3, 8:58 AM
Unknown Object (File)
Nov 2 2023, 5:40 PM
Unknown Object (File)
Oct 27 2023, 1:52 AM
Unknown Object (File)
Oct 19 2023, 2:19 PM
Unknown Object (File)
Oct 19 2023, 3:16 AM
Unknown Object (File)
Jul 21 2023, 5:07 AM
Unknown Object (File)
Jul 17 2023, 10:24 PM
Unknown Object (File)
Jul 17 2023, 5:00 AM
View All 20 Files
Subscribers
imp

Details

Reviewers
kib
Commits
rS345513: Reject F_SETLK_REMOTE commands when sysid == 0.
Summary

sysid 0 is the local system. Without this check, it's possible to
trigger the KASSERT in lf_clearremotesys(). Note that F_SETLK_REMOTE
is only available to privileged users.

I removed the comment about a temporary API because it's been in FreeBSD
for over 10 years and is used by some tests.

Diff Detail

Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 23289
Build 22321: arc lint + arc unit

Event Timeline

markj created this revision.Mar 25 2019, 4:32 PM
Harbormaster completed remote builds in B23289: Diff 55433.Mar 25 2019, 4:32 PM
markj added a reviewer: kib.Mar 25 2019, 4:33 PM
kib added a comment.Mar 25 2019, 4:57 PM

Is there a check that F_REMOTE has l_sysid != 0 ? I cannot find it, if any. We require PRIV_NFS_LOCKD for it, but I think slightly less trust would be due.

markj added a comment.Mar 25 2019, 7:26 PM
In D19702#422050, @kib wrote:

Is there a check that F_REMOTE has l_sysid != 0 ? I cannot find it, if any. We require PRIV_NFS_LOCKD for it, but I think slightly less trust would be due.

You mean, in the lockf layer? I don't think so. I don't quite understand why NLM is setting l_sysid = 0 in nlm_getlock().

kib added a comment.Mar 25 2019, 8:11 PM
In D19702#422083, @markj wrote:
In D19702#422050, @kib wrote:

Is there a check that F_REMOTE has l_sysid != 0 ? I cannot find it, if any. We require PRIV_NFS_LOCKD for it, but I think slightly less trust would be due.

You mean, in the lockf layer? I don't think so.

I mean, when fcntl(F_SETLK_REMOTE) in kern_descrip.c

I don't quite understand why NLM is setting l_sysid = 0 in nlm_getlock().

It seems to only do that for F_GETLK implementation, where userspace does not need to see l_sysid anyway.

markj updated this revision to Diff 55438.Mar 25 2019, 8:19 PM

Make the check more strict: disallow sysid == 0 for any F_REMOTE request.

Harbormaster completed remote builds in B23294: Diff 55438.Mar 25 2019, 8:19 PM
kib added inline comments.Mar 25 2019, 8:26 PM
sys/kern/kern_descrip.c
670

But now this version lost your original fix for F_UNLCKSYS ?

markj added inline comments.Mar 25 2019, 8:29 PM
sys/kern/kern_descrip.c
670

The new check catches this case too. We perform the check for all verbs instead of just F_UNLCKSYS.

kib accepted this revision.Mar 25 2019, 8:46 PM
This revision is now accepted and ready to land.Mar 25 2019, 8:46 PM
Closed by commit rS345513: Reject F_SETLK_REMOTE commands when sysid == 0. (authored by markj). · Explain WhyMar 25 2019, 9:39 PM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: imp. · View Herald TranscriptMar 25 2019, 9:39 PM

Revision Contents
Changeset List

PathSize
sys/
kern/
6 lines

Diff 55433

View Options

sys/kern/kern_descrip.c

Loading...