| Message ID | 1442562858-862-1-git-send-email-vapier@gentoo.org |
|---|---|
| State | Accepted, archived |
| Headers | show |
On Sep 18, 2015, at 01:54, Mike Frysinger <vapier@gentoo.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. Cheers, Andreas > URL: https://bugs.gentoo.org/550986 > Signed-off-by: Mike Frysinger <vapier@gentoo.org> > --- > util/subst.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/util/subst.c b/util/subst.c > index f36adb4..e4004c9 100644 > --- a/util/subst.c > +++ b/util/subst.c > @@ -370,7 +370,7 @@ int main(int argc, char **argv) > } > strcpy(newfn, outfn); > strcat(newfn, ".new"); > - fd = open(newfn, O_CREAT|O_TRUNC|O_RDWR, 0444); > + fd = open(newfn, O_CREAT|O_TRUNC|O_RDWR, 0644); > if (fd < 0) { > perror(newfn); > exit(1); > -- > 2.5.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-ext4" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 18 Sep 2015 10:52, Andreas Dilger wrote: > On Sep 18, 2015, at 01:54, Mike Frysinger <vapier@gentoo.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. if you want to try to "protect" people, then it needs to be a chmod after all the data has been written & closed. this is how it used to behave, but commit 2873927d15ffb9ee9ed0e2700791a0e519c715aa changed it. -mike
On Sep 18, 2015, at 12:08 PM, Mike Frysinger <vapier@gentoo.org> wrote: > > On 18 Sep 2015 10:52, Andreas Dilger wrote: >> On Sep 18, 2015, at 01:54, Mike Frysinger <vapier@gentoo.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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/util/subst.c b/util/subst.c index f36adb4..e4004c9 100644 --- a/util/subst.c +++ b/util/subst.c @@ -370,7 +370,7 @@ int main(int argc, char **argv) } strcpy(newfn, outfn); strcat(newfn, ".new"); - fd = open(newfn, O_CREAT|O_TRUNC|O_RDWR, 0444); + fd = open(newfn, O_CREAT|O_TRUNC|O_RDWR, 0644); if (fd < 0) { perror(newfn); exit(1);
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. URL: https://bugs.gentoo.org/550986 Signed-off-by: Mike Frysinger <vapier@gentoo.org> --- util/subst.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)