linux-kernel - Re: For review: open_by_name

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140318090007.3adee3d0@notabene.brown>
Date:	Tue, 18 Mar 2014 09:00:07 +1100
From:	NeilBrown <neilb@...e.de>
To:	"Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>
Cc:	"Aneesh Kumar K.V" <aneesh.kumar@...ux.vnet.ibm.com>,
	"linux-man@...r.kernel.org" <linux-man@...r.kernel.org>,
	Linux-Fsdevel <linux-fsdevel@...r.kernel.org>,
	lkml <linux-kernel@...r.kernel.org>,
	Andreas Dilger <adilger@...ger.ca>,
	Christoph Hellwig <hch@...radead.org>
Subject: Re: For review: open_by_name_at(2) man page

On Mon, 17 Mar 2014 16:57:29 +0100 "Michael Kerrisk (man-pages)"
<mtk.manpages@...il.com> wrote:

> Hi Aneesh, (and others)
> 
> Below is a man page I've written for name_to_handle_at(2) and
> open_by_name_at(2). Would you be willing to review it please,
> and let me know of any corrections/improvements?
> 
> Thanks,
> 
> Michael

Thanks for writing this Michael.  The fact that I can only find very small
points to comment on reflects the high quality...

> Otherwise,
> .IR dirfd
> must be a file descriptor that refers to a directory, and
  ^^^^^^^
> .I pathname
> is interpreted relative to that directory.

As you clarify later, "must be" is not correct.  Maybe this is just an issue
of style, in which case you should obviously keep a consistent style across
man pages, but to me it sounds wrong.  I would use "is generally" or similar.

> The
> .IR mountdirfd
> argument is a file descriptor for a directory under
> the mount point with respect to which
> .IR handle
> should be interpreted.

mountdirfd does not have to be for a directory.  It can be for any object in
the filesystem.  And I would say "in", not "under".
If /foo and /foo/bar are both mountpoints, and I want to look up a
filehandle for the filesystem mounted at /foo, then opening "/foo/bar"
wouldn't work even though /foo/bar is "under" /foo.  And opening "/foo" would
work even though "/foo" is not under "/foo/" (is it?).

  The
  .IR mountfd
  argument is a file descriptor for any object (file, directory, etc.) in the
  filesystem with respect to which
  .IR handle
  should be interpreted.

??

> .B ESTALE
> The specified
> .I handle
> is no longer valid.

ESTALE is also returned if the filesystem does not support file-handle ->
file mappings.
On filesystems which don't provide export_operations (/sys /proc ubifs
romfs cramfs nfs coda ... several others) name_to_handle_at will produce a
generic handle using the 32 bit inode and 32 bit i_generation.
open_by_name_at given this (or any) filehandle will fail with ESTALE.
I don't know how best to include this in the documentation.  Maybe a note
earlier noting that some filesystems do not support open_by_name_at(), and
you cannot programatically determine which do except by trying.
At the same time note that a file handle can become in valid if a file is
deleted or for any other reason as determined by the filesystem, and that the
error is the same as for when the filesystem doesn't support open_by_name_at.

> For example, one can use the device name in the fifth field of the
> .I mountinfo
> record to search for the corresponding device UUID via the symbolic links in
> .IR /dev/disks/by-uuid .
> (A more comfortable way of obtaining the UUID is to use the
> .\" e.g., http://stackoverflow.com/questions/6748429/using-libblkid-to-find-uuid-of-a-partition
> .BR libblkid (3)
> library, which uses the
> .I /sys
> filesystem to obtain the same information.)

Does it?  My understanding from "man libblkid" (it is a while since I've read
the code) is that it either uses info in /dev/disks/by-* or reads directly
from the block devices (maybe using /sys to find them?) and interprets the
superblock to extract a UUID.

> Now delete and re-create the file with the same inode number;
> .BR open_by_handle_at ()
> recognizes that the file referred to by the file handle no longer exists.
> 
> .in +4n
> .nf
> $ \fBstat \-\-printf="%i\\n" cecilia.txt\fP       # Display inode number 
> 4072121
> $ \fBecho 'Warum?' > cecilia.txt\fP
> $ \fBstat \-\-printf="%i\\n" cecilia.txt\fP       # Check inode number
> 4072121
> $ \fBsudo ./t_open_by_handle_at < fh\fP
> open_by_handle_at: Stale NFS file handle

Something is very wrong here.
   echo foo > somefile
does not "delete and re-create" the file.  It opens and truncates.
That operation should not invalidate the filehandle on any sane filesystem.

>     if (argc > 1)
>         mount_fd = open(argv[1], O_RDONLY | O_DIRECTORY);

O_DIRECTORY is not appropriate, as mentioned earlier.

Thanks,
NeilBrown

Download attachment "signature.asc" of type "application/pgp-signature" (829 bytes)