Re: [hackers] [sbase] [PATCH] mkfifo: Simplify -m handling

From: Richard Ipsum <richardipsum_AT_fastmail.co.uk>
Date: Thu, 13 Jun 2019 22:24:38 +0100

On Thu, Jun 13, 2019 at 01:51:05PM -0700, Michael Forney wrote:
> Rather than create the FIFO with incorrect permissions at first, then
> restore with chmod(2), just clear the umask when -m is specified, and
> pass the parsed mode to mkfifo(2).
> ---
> mkfifo.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/mkfifo.c b/mkfifo.c
> index 390381b..2470a09 100644
> --- a/mkfifo.c
> +++ b/mkfifo.c
> _AT_@ -14,14 +14,12 @@ usage(void)
> int
> main(int argc, char *argv[])
> {
> - mode_t mode = 0666, mask;
> - int mflag = 0, ret = 0;
> + mode_t mode = 0666;
> + int ret = 0;
>
> ARGBEGIN {
> case 'm':
> - mflag = 1;
> - mask = getumask();
> - mode = parsemode(EARGF(usage()), mode, mask);
> + mode = parsemode(EARGF(usage()), mode, umask(0));

I might be wrong but I think this will cause a subtle change in
behaviour since the umask is needed to correctly interpret the mode
string. One example that's documented in chmod(1p) is "-w" vs "a-w".
"a-w" removes all write perms, but "-w" removes only the permissions
that weren't filtered by the mask. This could explain why this
is currently being done in two separate steps.

That being said looking at mkfifo(1p) it says that if -m is present
then the value of mkfifo's mode argument is unspecified, but it
shouldn't be any less restrictive than the argument provided to -m.
So if I'm reading this correctly the current argument of
(S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH)
isn't strictly correct either since it could be less restrictive
than the argument provided to -m.

If I'm not mistaken in the second part then maybe the mode argument
to mkfifo should be 0, then apply the actual mode using chmod?

Hope this helps,
Richard
Received on Thu Jun 13 2019 - 23:24:38 CEST