Date:	Fri, 18 Sep 2015 14:36:34 -0600
From:	Andreas Dilger <adilger@...ger.ca>
To:	Mike Frysinger <vapier@...too.org>
Cc:	"linux-ext4@...r.kernel.org" <linux-ext4@...r.kernel.org>
Subject: Re: [PATCH e2fsprogs] subst: use 0644 perms

On Sep 18, 2015, at 12:08 PM, Mike Frysinger <vapier@...too.org> wrote:
> 
> On 18 Sep 2015 10:52, Andreas Dilger wrote:
>> On Sep 18, 2015, at 01:54, Mike Frysinger <vapier@...too.org> wrote:
>>> When running on NFS, opening files with 0444 perms for writing can
>>> sometimes fail.  Since there's no real reason for these files to be
>>> read-only, give the owner write permission.
>> 
>> Actually, there IS a reason for subst to make these files read-only. They are auto-generated and any edits to these files can be overwritten and lost if their origin files are modified. 
>> 
>> I'd lost edits to these auto generated files many time because they are the ones that "tags" or "cscope" will jump to when searching for symbols. 
>> 
>> There really isn't any reason for them to be writable, so the fact that you are getting an error trying to open them for writing is a hint that you are doing, or going to do, the wrong thing and the read-only nature of the file is preventing you from going down the wrong path. 
> 
> I think you misread my report.  this has nothing to do with people
> trying to modify the files after the fact.  NFS can (and sometimes
> does) throw an error at the time of the *open* call even if the file
> doesn't exist.

Seems like a bug in NFS.  At least I know the POSIX test suite requires
that to work (we had similar problems with Lustre that had to be fixed many years ago).  In fact, there is a special check for this in knfsd:


nfsd_permission(struct svc_rqst *rqstp, struct svc_export *exp,
                                        struct dentry *dentry, int acc)
{
    :
    :

    /*
     * The file owner always gets access permission for accesses that
     * would normally be checked at open time. This is to make
     * file access work even when the client has done a fchmod(fd, 0).
     *
     * However, `cp foo bar' should fail nevertheless when bar is
     * readonly. A sensible way to do this might be to reject all
     * attempts to truncate a read-only file, because a creat() call
     * always implies file truncation.
     * ... but this isn't really fair.  A process may reasonably call
     * ftruncate on an open file descriptor on a file with perm 000.
     * We must trust the client to do permission checking - using "ACCESS"
     * with NFSv3.
     */
    if ((acc & NFSD_MAY_OWNER_OVERRIDE) &&
        uid_eq(inode->i_uid, current_fsuid()))
            return 0;
    :
    :
}

Might be something that needs to be pursued with the kernel NFS folks?
That said, I'm not against fixing subst so that it works on your system.

> if you want to try to "protect" people, then it needs to be a chmod
> after all the data has been written & closed.

It that case, please update your patch to include something like:

                        if (verbose)
                                printf("Creating or replacing %s.\n", outfn);
+                       /* Avoid accidentally editing generated file. */
+                       (void)fchmod(out, 0444);
                        fclose(out);
                        if (old)
                                fclose(old);

>  this is how it used to
> behave, but commit 2873927d15ffb9ee9ed0e2700791a0e519c715aa changed it.


Cheers, Andreas





--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists