Re: Sanitizing (canonicalising) the block device name in mount_ffs ??

[Robert Elz <kre%munnari.OZ.AU@localhost>]

    Date:        Sat, 27 May 2023 20:15:01 +0200
    From:        "J. Hannken-Illjes" <hannken%mailbox.org@localhost>
    Message-ID:  <4365151A-B7D5-437A-A6A4-D586D5BF6277%mailbox.org@localhost>

  | The magic needed here is "blanket" from RUMPHIJACK(3) ...

Thanks.   That's something I would never have gotten to.

  | The attached diff should fix the tests.

It does.   Thanks.   Will you commit that, or do you want me to?

That makes the rest of my message moot (OBE) - no more changes
will be needed.

But I will answer a couple of the points raised by others, just
for completeness.


tlaronde%polynum.com@localhost said:
  | Isn't it because in order to be able to compare strings, the path has
  | to have an uniq (canonical) form, independent from the way the user
  | enters it?

It would, but I don't believe these strings are ever compared that way.

  | For example, at the user level, how mount(8) could compare,

mount(8) wasn't ever relevant, no change was proposed for that.
Just for mount_ffs (for now anyway, with some, but not all, of the
mount_xxx's perhaps later, for consistency), which won't be happening
now.   But those don't:
  | given only one argument, to what is in /etc/fstab without trying
  | first to give the pathname given some normal form?
do anything like that.

Normal use of the mount_xxx programs, via mount(8) wouldn't be affected
at all.

But I'd also agree with Mouse's reply - if you want to use mount(8) and
give just one arg, you should give the one that's in fstab, not something
else.   And yes, I do that all the time - but I always use the mount point,
which is the thing I care about, not the device name, which is kind of
arbitrary, and I'm never going to remember.   The mount point string was
always going to continue to be canonicalised, though I'm not sure anyone
should be relying upon that - just specify the absolute path to the mount
point.

tlaronde%polynum.com@localhost also said in a different message:
  | Since pathadj() was just sugar, calling realpath(3) (without really testing
  | the return) and emitting some messages, in a special case can you simply
  | "flatten" the thing i.e. replace the call to pathadj() by a call to
  | realpath(3)?

  | And then, there should be a code similar to what is done in src/sbin/mount/
  | mount.c: if canonical_path is NULL, try what the user passed:

That is, more or less exactly (though what mount(8) does is more complicated
than what mount_ffs (etc) need to do) what my possible change #4 was.

  | From a superficial knowledge, it seems to me that, eventually, the
  | __mount50() syscall has to be called with a canonical path, since the
  | syscall does no acrobatics with the path (and shall not be passed garbage). 

We don't generally refer to the syscalls by their versioned names, all we
care about is the current one (whatever its name) - the others are just for
old binaries that were built before some change was made to the API.  Nothing
is going to change anything about those any more.

But no, mount(2) does not require canonical names.   Certainly not
garbage (though that would normally just fail) but any path that reaches
the desired node in the filesystem tree should work.


mouse%Rodents-Montreal.ORG@localhost said:
  | It could also matter in that it would lead to a possibly-confusing
  | mount-from string in the mount table.

Yes, that's the kind of thing I was wondering about - the question (which
no longer needs answering) is whether or not that's just a cosmetic difference,
or would it matter somehow.

  | Also consider what happens if, say, a mount of /dev/foo is done in a chroot.
  |  [...]  I'm not sure whether I think that needs fixing either.

Nor am I, but that would have been unaffected by the possible changes
(which aren't happening now) - realpath() in a chroot returns the path
relative to the apparent root, not the external one.

  | And do what if pathadj fails?

Unaltered from what it is doing now (not what it did last week...)   That
is error message, and exit(1).   It has no way to return failure.

kre