Re: [PATCH] open.2: No need for /proc to make an O_TMPFILE file permanent

Re: [PATCH] open.2: No need for /proc to make an O_TMPFILE file permanent

[Michael Kerrisk (man-pages)]

Hello Adam,

On 9/26/18 1:09 AM, Adam Borowski wrote:
> In the example snippet, we already have the fd, thus there's no
> need to refer to the file by name.  And, /proc/ might be not mounted
> or not accessible.

Thanks! Patch applied.

Cheers,

Michael

> Noticed-by: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Adam Borowski <kilobyte@angband.pl>
> ---
>  man2/open.2 | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/man2/open.2 b/man2/open.2
> index 5d0ce66d8..1c775b6b3 100644
> --- a/man2/open.2
> +++ b/man2/open.2
> @@ -811,9 +811,7 @@ fd = open("/path/to/dir", O_TMPFILE | O_RDWR,
>  
>  /* File I/O on 'fd'... */
>  
> -snprintf(path, PATH_MAX,  "/proc/self/fd/%d", fd);
> -linkat(AT_FDCWD, path, AT_FDCWD, "/path/for/file",
> -                        AT_SYMLINK_FOLLOW);
> +linkat(fd, NULL, AT_FDCWD, "/path/for/file", AT_EMPTY_PATH);
>  .EE
>  .in
>  .IP
> 


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/

Re: [PATCH] open.2: No need for /proc to make an O_TMPFILE file permanent

[Michael Kerrisk (man-pages)]

Hello Joseph,

On 9/27/18 3:55 AM, Joseph C. Sible wrote:
> The old code doesn't require any capabilities, but your new code
> requires that the user have CAP_DAC_READ_SEARCH, due to the use of
> AT_EMPTY_PATH with linkat. (I personally think CAP_DAC_READ_SEARCH
> should be unnecessary and that f0cc6ffb should be reverted, but I
> digress.)
> 
> Joseph C. Sible
>
> On Tue, Sep 25, 2018 at 7:13 PM Adam Borowski <kilobyte@angband.pl> wrote:
>>
>> In the example snippet, we already have the fd, thus there's no
>> need to refer to the file by name.  And, /proc/ might be not mounted
>> or not accessible.
>>
>> Noticed-by: Theodore Ts'o <tytso@mit.edu>
>> Signed-off-by: Adam Borowski <kilobyte@angband.pl>
>> ---
>>  man2/open.2 | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/man2/open.2 b/man2/open.2
>> index 5d0ce66d8..1c775b6b3 100644
>> --- a/man2/open.2
>> +++ b/man2/open.2
>> @@ -811,9 +811,7 @@ fd = open("/path/to/dir", O_TMPFILE | O_RDWR,
>>
>>  /* File I/O on 'fd'... */
>>
>> -snprintf(path, PATH_MAX,  "/proc/self/fd/%d", fd);
>> -linkat(AT_FDCWD, path, AT_FDCWD, "/path/for/file",
>> -                        AT_SYMLINK_FOLLOW);
>> +linkat(fd, NULL, AT_FDCWD, "/path/for/file", AT_EMPTY_PATH);
>>  .EE
>>  .in
>>  .IP

Thanks for pointing that out. I've applied Adam's patch, and
added a patch noting the alternative (which was previously
shown in the manual page):

diff --git a/man2/open.2 b/man2/open.2
index db09389a1..b47241b22 100644
--- a/man2/open.2
+++ b/man2/open.2
@@ -812,6 +812,16 @@ fd = open("/path/to/dir", O_TMPFILE | O_RDWR,
 /* File I/O on 'fd'... */
 
 linkat(fd, NULL, AT_FDCWD, "/path/for/file", AT_EMPTY_PATH);
+
+/* If the caller doesn't have the CAP_DAC_READ_SEARCH
+   capability (needed to use AT_EMPTY_PATH with linkat(2)),
+   and there is a proc(5) filesystem mounted, then the
+   linkat(2) call above can be replaced with:
+
+snprintf(path, PATH_MAX,  "/proc/self/fd/%d", fd);
+linkat(AT_FDCWD, path, AT_FDCWD, "/path/for/file",
+                        AT_SYMLINK_FOLLOW);
+*/
 .EE
 .in
 .IP

Cheers,

Michael


-- 
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/