Re: RFC: fixing PR#282995

From: <rb_at_gid.co.uk>
Date: Thu, 28 Nov 2024 15:48:48 UTC

> On 28 Nov 2024, at 15:04, Rick Macklem <rick.macklem@gmail.com> wrote:
> 
> On Thu, Nov 28, 2024 at 4:36 AM Bob Bishop <rb@gid.co.uk> wrote:
>> 
>> Hi,
>> 
>>> On 27 Nov 2024, at 21:56, Rick Macklem <rick.macklem@gmail.com> wrote:
>>> 
>>> Hi,
>>> 
>>> PR#282995 reports that the "-alldirs" export option is broken,
>>> since it allows an export where the directory path is not a mount point.
>>> 
>>> I'll admit I did not recall this semantic for -alldirs and I now see it is only
>>> documented in the "Examples" section of exports(5).
>>> 
>>> Looking at the code, it appears this was broken between releng1 and
>>> releng2.0 (about 30years ago) when the call to mount(2) in mountd.c
>>> was changed from using the path in the exports line to using f_mntonname.
>>> (The check for "it is a mount point" depended on mount(2) failing because
>>> the path was not a mount point.)
>>> 
>>> I do believe the semantic is a useful one,
>> 
>> Why?
> Suppose /cdrom is where a CD is mounted sometimes.
> If this is exported when the CD is not mounted, it will export
> the "/" file system.
> --> This export is probably not what the sysadmin wanted.
>      mountd does now generate a warning about this, which
>      was how the exporter spotted the bug.
> For example (the line in /etc/exports):
> /cdrom -alldirs
> will export "/" to "the world" if /cdrom is not mounted.

I will agree that is undesirable.

> The example in the exports(5) man page claims the export
> line will fail, so "/" would not be exported. This seems like
> a better semantic to me.

It’s certainly safer but will cause client mounts to fail as well. It would be nicer to export an empty directory.

> rick
> 
>> 
>>> although making it that way
>>> after 30years might be construed as a POLA violation?
>>> 
>>> So, what do others think I should do with this?
>>> (A) - Patch mountd to enforce the "must be a mount point when -alldirs
>>>       is specified, plus update exports(5) to state this semantic clearly.
>>> or
>>> (B) - Patch mountd so that it enforces "must be a mount point when -alldirs
>>>       is specified, but only enabled via a new mountd command line option.
>>>       --> ie. Leave the default as not enforced, but allow enforcement based
>>>                 on a new mountd option.
>>>            - Document this in both exports(5) and mountd(8).
>>> or
>>> ???
>> 
>> (C) - Patch mountd so that it enforces "must be a mount point when -alldirs
>>       is specified, but provide a new mountd command line option to restore the old behaviour.
>>       --> ie. Default as enforced, but allow an override based  on a new mountd option.
>>            - Document this in both exports(5) and mountd(8).
>> 
>> I think that (A) is too POLA-unfriendly.
>> 
>>> Thanks in advance for your comments, rick
>>> 
>> 
>> --
>> Bob Bishop
>> rb@gid.co.uk
>> 
>> 
>> 
>> 
> 

--
Bob Bishop
rb@gid.co.uk