About pseudo's chmod
Hi,
When run "chmod 0444 <file>" under pseudo, it would always adds write permission for real file (and w + x for dir), which means that it runs as "chmod 0644 <file>". It does this on real file, not record this on pseudo's database. Here are the code from pseudo:
/* Root can read and write files, and enter directories which have no
* read, write, or execute permissions. (But can't execute files without
* execute permissions!)
*
* A non-root user can't.
*
* When doing anything which actually writes to the filesystem, we add in
* the user read/write/execute bits. When storing to the database, though,
* we mask out any such bits which weren't in the original mode.
*/
#define PSEUDO_FS_MODE(mode, isdir) (((mode) | S_IRUSR | S_IWUSR | ((isdir) ? S_IXUSR : 0)) & ~(S_IWGRP | S_IWOTH)) #define PSEUDO_DB_MODE(fs_mode, user_mode) (((fs_mode) & ~0722) | ((user_mode & 0722)))
It has a side effect for -dbg pkgs if the source files foo.c's mode is 0444:
1) bitbake foo
2) Edit rpm-native
3) bitbake foo
After the first bitake foo, we will see that foo.c in foo-dbg is 0444, but after the second bitbake foo, foo.c in foo-dbg will be 0644, because the first build has changed src file foo.c's mode to 0644, this is incorrect.
I have two suggestions on it:
1) Don't add more permissions when chmod(e.g., don't change 0444 -> 0644), The user can add it clearly if a file/dir really needs that.
2) This mainly affects do_package task AFAIK, the code is:
if not cpath.islink(file):
os.link(file, fpath)
fstat = cpath.stat(file)
os.chmod(fpath, fstat.st_mode)
os.chown(fpath, fstat.st_uid, fstat.st_gid)
Another solution is checking mode before run chmod, if we really need run chmod, then copy the file rather than link.
Any suggestion is appreciated.
The following recipes in oe-core have this issue:
blktool
coreutils
e2fsprogs
gnutls
guile
gzip
less
lsof
mtools
opensp
parted
screen
tcp-wrappers
Signature · 3 lines
Thanks Robert
Quoted 27 lines · Robert Yang
Hi,
When run "chmod 0444 <file>" under pseudo, it would always adds write permission for real file (and w + x for dir), which means that it runs as "chmod 0644 <file>". It does this on real file, not record this on pseudo's database. Here are the code from pseudo:
/* Root can read and write files, and enter directories which have no
* read, write, or execute permissions. (But can't execute files without
* execute permissions!)
*
* A non-root user can't.
*
* When doing anything which actually writes to the filesystem, we add in
* the user read/write/execute bits. When storing to the database, though,
* we mask out any such bits which weren't in the original mode. */
#definePSEUDO_FS_MODE(mode, isdir)(((mode) |S_IRUSR|S_IWUSR| ((isdir) ?S_IXUSR: 0)) & ~(S_IWGRP |S_IWOTH))
#definePSEUDO_DB_MODE(fs_mode, user_mode)(((fs_mode) & ~0722) | ((user_mode& 0722)))It has a side effect for -dbg pkgs if the source files foo.c's mode is 0444:
1) bitbake foo
2) Edit rpm-native
3) bitbake foo
Quoted 7 lines
After the first bitake foo, we will see that
foo.cin foo-dbg is 0444, but after the second bitbake foo,foo.cin foo-dbg will be 0644, because the first build has changed src file foo.c's mode to 0644, this is incorrect.I have two suggestions on it:
1) Don't add more permissions whenchmod(e.g., don't change 0444 -> 0644), The user can add it clearly if a file/dir really needs that.
As noted above, we have to adjust the permissions to writable, or we can not make various changes later. When working as the 'root' user, permissions are basically ignored. So a non-writable file is still writable. The only way to emulate this is to make the actual file writable.
I don't understand how/why on a second run the 0644 is showing up though. Unless the pseudo database is wiped -- or the debug commands are not clearing the split directories before writing into them, it should be creating new files with the new pseudo database that follows the same semantics.
Quoted 27 lines
2) This mainly affects
do_packagetask AFAIK, the code is:
if notcpath.islink(file):
os.link(file, fpath)
fstat =cpath.stat(file)os.chmod(fpath, fstat.st_mode)os.chown(fpath, fstat.st_uid, fstat.st_gid)Another solution is checking mode before run chmod, if we really need run chmod, then copy the file rather than link.
Any suggestion is appreciated.
The following recipes in oe-core have this issue:
blktool
coreutils
e2fsprogs
gnutls
guile
gzip
less
lsof
mtools
opensp
parted
screen
tcp-wrappers
Quoted 41 lines · Mark Hatle
Quoted 27 lines · Robert Yang
Hi,
When run "chmod 0444 <file>" under pseudo, it would always adds write permission for real file (and w + x for dir), which means that it runs as "chmod 0644 <file>". It does this on real file, not record this on pseudo's database. Here are the code from pseudo:
/* Root can read and write files, and enter directories which have no
* read, write, or execute permissions. (But can't execute files without
* execute permissions!)
*
* A non-root user can't.
*
* When doing anything which actually writes to the filesystem, we add in
* the user read/write/execute bits. When storing to the database, though,
* we mask out any such bits which weren't in the original mode. */
#definePSEUDO_FS_MODE(mode, isdir)(((mode) |S_IRUSR|S_IWUSR| ((isdir) ?S_IXUSR: 0)) & ~(S_IWGRP |S_IWOTH))
#definePSEUDO_DB_MODE(fs_mode, user_mode)(((fs_mode) & ~0722) | ((user_mode& 0722)))It has a side effect for -dbg pkgs if the source files foo.c's mode is 0444:
1) bitbake foo
2) Edit rpm-native
3) bitbake fooQuoted 7 lines
After the first bitake foo, we will see that
foo.cin foo-dbg is 0444, but after the second bitbake foo,foo.cin foo-dbg will be 0644, because the first build has changed src file foo.c's mode to 0644, this is incorrect.I have two suggestions on it:
1) Don't add more permissions whenchmod(e.g., don't change 0444 -> 0644), The user can add it clearly if a file/dir really needs that.As noted above, we have to adjust the permissions to writable, or we can not make various changes later. When working as the 'root' user, permissions are basically ignored. So a non-writable file is still writable. The only way to emulate this is to make the actual file writable.
Quoted 5 lines
I don't understand how/why on a second run the 0644 is showing up though. Unless the pseudo database is wiped -- or the debug commands are not clearing the split directories before writing into them, it should be creating new files with the new pseudo database that follows the same semantics.
I had talked with Mark via IM, why it gets 0644 is because of the code. fstat = cpath.stat(file) in package.bbclass, this command would get mode from the real file rather than pseudo's database, and this is expected. And in the second run, the file's mode had been changed from 0444 to 0644 in the first run, so it would get 0644.
The real problem might be that filesystem such as ext4 doesn't allow different items of hardlinks have different permissions.
// Robert
Quoted 29 lines
Quoted 27 lines
2) This mainly affects
do_packagetask AFAIK, the code is:
if notcpath.islink(file):
os.link(file, fpath)
fstat =cpath.stat(file)os.chmod(fpath, fstat.st_mode)os.chown(fpath, fstat.st_uid, fstat.st_gid)Another solution is checking mode before run chmod, if we really need run chmod, then copy the file rather than link.
Any suggestion is appreciated.
The following recipes in oe-core have this issue:
blktool
coreutils
e2fsprogs
gnutls
guile
gzip
less
lsof
mtools
opensp
parted
screen
tcp-wrappers
Quoted 36 lines · Mark Hatle
Quoted 27 lines · Robert Yang
Hi,
When run "chmod 0444 <file>" under pseudo, it would always adds write permission for real file (and w + x for dir), which means that it runs as "chmod 0644 <file>". It does this on real file, not record this on pseudo's database. Here are the code from pseudo:
/* Root can read and write files, and enter directories which have no
* read, write, or execute permissions. (But can't execute files without
* execute permissions!)
*
* A non-root user can't.
*
* When doing anything which actually writes to the filesystem, we add in
* the user read/write/execute bits. When storing to the database, though,
* we mask out any such bits which weren't in the original mode. */
#definePSEUDO_FS_MODE(mode, isdir)(((mode) |S_IRUSR|S_IWUSR| ((isdir) ?S_IXUSR: 0)) & ~(S_IWGRP |S_IWOTH))
#definePSEUDO_DB_MODE(fs_mode, user_mode)(((fs_mode) & ~0722) | ((user_mode& 0722)))It has a side effect for -dbg pkgs if the source files foo.c's mode is 0444:
1) bitbake foo
2) Edit rpm-native
3) bitbake fooQuoted 7 lines
After the first bitake foo, we will see that
foo.cin foo-dbg is 0444, but after the second bitbake foo,foo.cin foo-dbg will be 0644, because the first build has changed src file foo.c's mode to 0644, this is incorrect.I have two suggestions on it:
1) Don't add more permissions whenchmod(e.g., don't change 0444 -> 0644), The user can add it clearly if a file/dir really needs that.
Quoted 6 lines
As noted above, we have to adjust the permissions to writable, or we can not make various changes later. When working as the 'root' user, permissions are basically ignored. So a non-writable file is still writable. The only way to emulate this is to make the actual file writable.
I don't understand how/why on a second run the 0644 is showing up though.
Hi Mark,
It got 0644 but not 0444 in the second build was because pseudo's unlink doesn't take core of hard links, for example:
$ umask 0022 $ touch file1 $ ln file1 file2 $ chmod 0777 file1 $ ls -l file1 file2 We can see that both file1 and file2's mode is 0777.
But if we remove file1:
$ rm -f file1 $ ls file2 Now file2's mode is 0644 since the info had been removed from database.
After talked with RP online, there isn't any file systems that can support different modes on different references for the same inode, so pseudo's chmod should update all the references' mode, in another word, it should not remove the info from database if links count is greater than 1.
Here is a patch for pseudo to fix the problem, I'm testing it locally now, will send out sooner:
ports/unix/guts/unlinkat.c +4 -1
@@ -36,8 +36,9 @@if (rc ==-1 ) {return rc; } + msg =pseudo_client_op (OP_MAY_UNLINK ,0 ,-1 , dirfd, path, &buf); -if (msg && msg->result ==RESULT_SUCCEED ) +if (msg && msg->result ==RESULT_SUCCEED && buf.st_nlink ==1 ) old_db_entry =1 ;#ifdef PSEUDO_NO_REAL_AT_FUNCTIONS rc =real_unlink (path);
@@ -52,6 +53,8 @@ }else {pseudo_client_op (OP_DID_UNLINK ,0 ,-1 ,-1 , path, &buf); } + }else if (buf.st_nlink >1 ) { +pseudo_debug (PDBGF_FILE ,"not remove ino %lu from database since its links count is greater than 1.\n" , buf.st_ino); }else { pseudo_debug(PDBGF_FILE, "unlink on <%s>, not in database, no
effect.\n", path);
}// Robert
Quoted 33 lines
Unless the pseudo database is wiped -- or the debug commands are not clearing the split directories before writing into them, it should be creating new files with the new pseudo database that follows the same semantics.
Quoted 27 lines
2) This mainly affects
do_packagetask AFAIK, the code is:
if notcpath.islink(file):
os.link(file, fpath)
fstat =cpath.stat(file)os.chmod(fpath, fstat.st_mode)os.chown(fpath, fstat.st_uid, fstat.st_gid)Another solution is checking mode before run chmod, if we really need run chmod, then copy the file rather than link.
Any suggestion is appreciated.
The following recipes in oe-core have this issue:
blktool
coreutils
e2fsprogs
gnutls
guile
gzip
less
lsof
mtools
opensp
parted
screen
tcp-wrappers
Quoted 57 lines · Robert Yang
Quoted 36 lines · Mark Hatle
Quoted 27 lines · Robert Yang
Hi,
When run "chmod 0444 <file>" under pseudo, it would always adds write permission for real file (and w + x for dir), which means that it runs as "chmod 0644 <file>". It does this on real file, not record this on pseudo's database. Here are the code from pseudo:
/* Root can read and write files, and enter directories which have no
* read, write, or execute permissions. (But can't execute files without
* execute permissions!)
*
* A non-root user can't.
*
* When doing anything which actually writes to the filesystem, we add in
* the user read/write/execute bits. When storing to the database, though,
* we mask out any such bits which weren't in the original mode. */
#definePSEUDO_FS_MODE(mode, isdir)(((mode) |S_IRUSR|S_IWUSR| ((isdir) ?S_IXUSR: 0)) & ~(S_IWGRP |S_IWOTH))
#definePSEUDO_DB_MODE(fs_mode, user_mode)(((fs_mode) & ~0722) | ((user_mode& 0722)))It has a side effect for -dbg pkgs if the source files foo.c's mode is 0444:
1) bitbake foo
2) Edit rpm-native
3) bitbake fooQuoted 7 lines
After the first bitake foo, we will see that
foo.cin foo-dbg is 0444, but after the second bitbake foo,foo.cin foo-dbg will be 0644, because the first build has changed src file foo.c's mode to 0644, this is incorrect.I have two suggestions on it:
1) Don't add more permissions whenchmod(e.g., don't change 0444 -> 0644), The user can add it clearly if a file/dir really needs that.Quoted 6 lines
As noted above, we have to adjust the permissions to writable, or we can not make various changes later. When working as the 'root' user, permissions are basically ignored. So a non-writable file is still writable. The only way to emulate this is to make the actual file writable.
I don't understand how/why on a second run the 0644 is showing up though.
Hi Mark,
It got 0644 but not 0444 in the second build was because pseudo's unlink doesn't take core of hard links, for example:
$ umask 0022 $ touch file1 $ ln file1 file2 $ chmod 0777 file1 $ ls -l file1 file2 We can see that both file1 and file2's mode is 0777.
Quoted 7 lines
But if we remove file1:
$ rm -f file1 $ ls file2 Now file2's mode is 0644 since the info had been removed from database.After talked with RP online, there isn't any file systems that can support different modes on different references for the same inode, so pseudo's
Here should be: "there isn't any file systems can ... *as far as we know*"
// Robert
Quoted 68 lines
chmod should update all the references' mode, in another word, it should not remove the info from database if links count is greater than 1.
Here is a patch for pseudo to fix the problem, I'm testing it locally now, will send out sooner:
ports/unix/guts/unlinkat.c +4
-1@@ -36,8 +36,9 @@if (rc ==-1 ) {return rc; } + msg =pseudo_client_op (OP_MAY_UNLINK ,0 ,-1 , dirfd, path, &buf); -if (msg && msg->result ==RESULT_SUCCEED ) +if (msg && msg->result ==RESULT_SUCCEED && buf.st_nlink ==1 ) old_db_entry =1 ;#ifdef PSEUDO_NO_REAL_AT_FUNCTIONS rc =real_unlink (path);@@ -52,6 +53,8 @@ }else {pseudo_client_op (OP_DID_UNLINK ,0 ,-1 ,-1 , path, &buf); } + }else if (buf.st_nlink >1 ) { + pseudo_debug(PDBGF_FILE, "not remove ino %lu from database since its links count is greater than 1.\n", buf.st_ino); } else { pseudo_debug(PDBGF_FILE, "unlink on <%s>, not in database, noeffect.\n", path); }// Robert
Quoted 33 lines
Unless the pseudo database is wiped -- or the debug commands are not clearing the split directories before writing into them, it should be creating new files with the new pseudo database that follows the same semantics.
Quoted 27 lines
2) This mainly affects
do_packagetask AFAIK, the code is:
if notcpath.islink(file):
os.link(file, fpath)
fstat =cpath.stat(file)os.chmod(fpath, fstat.st_mode)os.chown(fpath, fstat.st_uid, fstat.st_gid)Another solution is checking mode before run chmod, if we really need run chmod, then copy the file rather than link.
Any suggestion is appreciated.
The following recipes in oe-core have this issue:
blktool
coreutils
e2fsprogs
gnutls
guile
gzip
less
lsof
mtools
opensp
parted
screen
tcp-wrappers
Quoted 9 lines · Robert Yang
It got 0644 but not 0444 in the second build was because pseudo's unlink doesn't take core of hard links, for example:
$ umask 0022 $ touch file1 $ ln file1 file2 $ chmod 0777 file1 $ ls -l file1 file2 We can see that both file1 and file2's mode is 0777.
Quoted 4 lines
But if we remove file1:
$ rm -f file1 $ ls file2 Now file2's mode is 0644 since the info had been removed from database.
I don't get that result. Before the remove, I see:
sqlite> select * from files;
1|/home/seebs/pseudo/f1|2054|2757274|0|0|33279|0|0 2|/home/seebs/pseudo/f2|2054|2757274|0|0|33279|0|0
So both files have the correct information stored for them. This is because chmod-type operations on files update everything with the same device and inode.
Quoted 3 lines
After talked with RP online, there isn't any file systems that can support different modes on different references for the same inode, so pseudo's chmod should update all the references' mode
Yes.
Quoted 2 lines
in another word, it should
not remove the info from database if links count is greater than 1.
This doesn't seem right to me. I can't produce any failures from the current code that match the behavior you're describing; updates like chmod always propogate to all the links.
On the other hand, if f1 *already exists*, and was not being tracked by pseudo, then we do indeed see a similar problem. The underlying file's mode is actually changed.
I do not think it would be a good idea at all to stop removing entries from the database when they become invalid. It seems to me that if we're hard linking to a file inside pseudo, we should probably be tracking that file. Although if we track the file, now we'll just continue to see it as mode 777, because we changed the mode of the file.
Consider, for a moment, what happens if you do this *without* pseudo involved. Unpack an archive containing a file "foo", mode 444.
$ ln foo bar $ chmod 777 bar $ rm bar $ ls -l foo
You'll find that foo's mode is now 0777. So it'd change either way; the
difference is that, when pseudo's doing this, it masks out group and
other write bits on the filesystem. (Because we don't want other people to be able to overwrite things in the build tree just because they'd be writeable on the target filesystem.)
-s
Hi Peter,
Thanks for the reply.
Quoted 18 lines · Seebs
Quoted 9 lines · Robert Yang
It got 0644 but not 0444 in the second build was because pseudo's unlink doesn't take core of hard links, for example:
$ umask 0022 $ touch file1 $ ln file1 file2 $ chmod 0777 file1 $ ls -l file1 file2 We can see that both file1 and file2's mode is 0777.Quoted 4 lines
But if we remove file1:
$ rm -f file1 $ ls file2 Now file2's mode is 0644 since the info had been removed from database.I don't get that result. Before the remove, I see:
Quoted 7 lines
sqlite> select * from files;
1|/home/seebs/pseudo/f1|2054|2757274|0|0|33279|0|0 2|/home/seebs/pseudo/f2|2054|2757274|0|0|33279|0|0So both files have the correct information stored for them. This is because chmod-type operations on files update everything with the same device and inode.
Sorry, the steps were wrong, they should be:
1) Out of pseudo:
$ umask 0022 $ touch file1 2) Under pseudo: $ ln file1 file2 $ chmod 777 file2 $ ls -l file1 file2 We can see that both file1 and file2's mode is 0777. But if we remove file2: $ rm -f file2 $ ls file1 Now file1's permission is 0755, not 0777 or 0644, it should be 0777 here.
Quoted 14 lines
Quoted 4 lines
After talked with RP online, there isn't any file systems that can support different modes on different references for the same inode, so pseudo's chmod should update all the references' mode
Yes.
Quoted 2 lines
in another word, it should
not remove the info from database if links count is greater than 1.This doesn't seem right to me. I can't produce any failures from the current code that match the behavior you're describing; updates like chmod always propogate to all the links.
Quoted 7 lines
On the other hand, if f1 *already exists*, and was not being tracked by pseudo, then we do indeed see a similar problem. The underlying file's mode is actually changed.
I do not think it would be a good idea at all to stop removing entries from the database when they become invalid. It seems to me that if we're hard linking to a file inside pseudo, we should probably be tracking that file. Although if we
Yes, I agree that we need track the src file when hard link.
// Robert
Quoted 18 lines
track the file, now we'll just continue to see it as mode 777, because we changed the mode of the file.
Consider, for a moment, what happens if you do this *without* pseudo involved. Unpack an archive containing a file "foo", mode 444.
$ ln foo bar $ chmod 777 bar $ rm bar $ ls -l fooYou'll find that foo's mode is now 0777. So it'd change either way; the difference is that, when pseudo's doing this, it masks out group and other write bits on the filesystem. (Because we don't want other people to be able to overwrite things in the build tree just because they'd be writeable on the target filesystem.)
-s
Quoted 14 lines · Robert Yang
Sorry, the steps were wrong, they should be:
1) Out of pseudo:$ umask 0022 $ touch file1 2) Under pseudo: $ ln file1 file2 $ chmod 777 file2 $ ls -l file1 file2 We can see that both file1 and file2's mode is 0777. But if we remove file2: $ rm -f file2 $ ls file1 Now file1's permission is 0755, not 0777 or 0644, it should be 0777 here.
The short answer is: If you aren't tracking a file in pseudo, we don't make promises about its behavior. Normally, we don't touch them, but if there's hard links to them that are being touched, well. And having a hard link that is tracked, and another that isn't, is probably impossible to support. I definitely don't want to keep database entries for files that have been deleted, that way lies madness and possible database corruption; for instance, if we do that, and you make a new file of the same type, it'll show up as having those permissions, with only a path-mismatch warning in the database to suggest what went wrong.
I would say that the probable correct answer is either "make the original file be tracked" or "don't use hard links in this case".
Note that, under older pseudo, the file would have ended up 0777. The behavior of silently masking out 022 from underlying filesystem permissions is entirely intentional. During some debugging quite a while back, we discovered a quirk in oe-core, plus a strange behavior of GNU tar, which between them resulted in some sstage directories getting
unpacked with mode 0777.
And we *really* don't want the build system generating files which other users can modify, especially not in stuff that's intended to go into a root filesystem. So stripping out those bits in the underlying filesystem is intentional.
If you were actually root, yes, the original file would have its mode changed to 0777. But we should never be *caring* about the mode bits on raw files outside of pseudo, except that we want them to allow owner
write permissions and not allow group or other write permissions. If the
file's permissions matter to the build system or generated stuff, the
file should be tracked by pseudo.
-s
Quoted 25 lines · Seebs
Quoted 13 lines · Robert Yang
Sorry, the steps were wrong, they should be:
1) Out of pseudo:$ umask 0022 $ touch file1 2) Under pseudo: $ ln file1 file2 $ chmod 777 file2 $ ls -l file1 file2 We can see that both file1 and file2's mode is 0777. But if we remove file2: $ rm -f file2 $ ls file1 Now file1's permission is 0755, not 0777 or 0644, it should be 0777 here.The short answer is: If you aren't tracking a file in pseudo, we don't make promises about its behavior. Normally, we don't touch them, but if there's hard links to them that are being touched, well. And having a hard link that is tracked, and another that isn't, is probably impossible to support. I definitely don't want to keep database entries for files that have been deleted, that way lies madness and possible database corruption; for instance, if we do that, and you make a new file of the same type, it'll show up as having those permissions, with only a path-mismatch warning in the database to suggest what went wrong.
Quoted 2 lines
I would say that the probable correct answer is either "make the original file be tracked" or "don't use hard links in this case".
Hi Peter,
How about we track the src when hardlink, for example:
$ ln oldpath newpath
Track both oldpath and newpath. The patch for pseudo is:
ports/unix/guts/linkat.c +4 -0
@@ -62,6 +62,10 @@* if the thing linked is a symlink. */ pseudo_client_op (OP_LINK ,0 ,-1 ,-1 , newpath, &buf); +/* +* Track oldpath in case it isn't tracked. +*/ +pseudo_client_op (OP_LINK ,0 ,-1 ,-1 , oldpath, &buf); errno = save_errno;
// Robert
Quoted 21 lines
Note that, under older pseudo, the file would have ended up 0777. The behavior of silently masking out 022 from underlying filesystem permissions is entirely intentional. During some debugging quite a while back, we discovered a quirk in oe-core, plus a strange behavior of
GNU tar, which between them resulted in some sstage directories getting unpacked with mode 0777.And we *really* don't want the build system generating files which other users can modify, especially not in stuff that's intended to go into a root filesystem. So stripping out those bits in the underlying filesystem is intentional.
If you were actually root, yes, the original file would have its mode changed to 0777. But we should never be *caring* about the mode bits on
raw files outside of pseudo, except that we want them to allow owner write permissions and not allow group or other write permissions. If the file's permissions matter to the build system or generated stuff, the file should be tracked by pseudo.-s
Quoted 1 line · Robert Yang
How about we track the src when hardlink, for example:
I don't think I like this.
Fundamentally, the problem here is making a hard link to a file outside of pseudo, and then modifying the file.
Consider, if you will, what happens if you replace "chmod 0777 file2" with:
# echo "# last line" >> file2
The problem here isn't pseudo's tracking; it's that we're making a hard link to a file, modifying the hard link, deleting the hard link, and expecting the original file to have the same attributes it had before this all happened.
-s
Quoted 12 lines · Seebs
Quoted 1 line · Robert Yang
How about we track the src when hardlink, for example:
I don't think I like this.
Fundamentally, the problem here is making a hard link to a file outside
of pseudo, and then modifying the file.Consider, if you will, what happens if you replace "chmod 0777 file2" with:
Quoted 7 lines
# echo "# last line" >> file2The problem here isn't pseudo's tracking; it's that we're making a hard
link to a file, modifying the hard link, deleting the hard link, and expecting the original file to have the same attributes it had before this all happened.
No, we're actually expecting it to retain the mode it was given via the hardlink under pseudo.
This is what a real world system would do and in this case we could track it via pseudo since pseudo is loaded when the hardlink is created. It would be unreasonable for pseudo to track all hardlinks but tracking ones created under it does seem reasonable?
Cheers,
Richard
Quoted 7 lines · Richard Purdie
No, we're actually expecting it to retain the mode it was given via the hardlink under pseudo.
This is what a real world system would do and in this case we could track it via pseudo since pseudo is loaded when the hardlink is created. It would be unreasonable for pseudo to track all hardlinks but tracking ones created under it does seem reasonable?
Hmm. Well, strictly speaking, the link created under pseudo *does* get tracked. Hmm. But an implicit request to track also the thing linked to is not a horrible idea. Although you'd still be able to beat it:
$ touch file1 $ ln file1 file2 $ pseudo # ln file2 file3 # chmod 777 file3 # rm file2 file3 # ls -l file1
The general case of "find everything this link also refers to" is clearly out of scope. That said... Hmm. I think my main feeling is, if we want to link to the file, and we want the changes to the linked file to
survive, we should probably either create that file under pseudo, or explicitly claim it with pseudo when we start wanting to do the tracking. (You can trivially do this to a tree with chown -R root tree).
-s
Quoted 26 lines · Seebs
Quoted 9 lines · Richard Purdie
No, we're actually expecting it to retain the mode it was given via the
hardlink under pseudo.This is what a real world system would do and in this case we could track it via pseudo since pseudo is loaded when the hardlink is created. It would be unreasonable for pseudo to track all hardlinks but
tracking ones created under it does seem reasonable?Hmm. Well, strictly speaking, the link created under pseudo *does* get
tracked. Hmm. But an implicit request to track also the thing linked to
is not a horrible idea. Although you'd still be able to beat it:$ touch file1 $ ln file1 file2 $ pseudo # ln file2 file3 # chmod 777 file3 # rm file2 file3 # ls -l file1
Quoted 2 lines
The general case of "find everything this link also refers to" is clearly out of scope.
Agreed.
Quoted 6 lines
That said... Hmm. I think my main feeling is, if we want
to link to the file, and we want the changes to the linked file to survive, we should probably either create that file under pseudo, or explicitly claim it with pseudo when we start wanting to do the tracking.
(You can trivially do this to a tree with chown -R root tree).
The trouble is that for speed, we do create trees of hardlinked files and play games with those and sstate amongst other things. Its obviously faster to do this than make physical copies of the files. Given what I know of the code paths, I suspect that tracking the source of a hardlink would make life much easier for us. Obviously we can go and start adding "chown -R" calls everywhere but that seems a little ugly to me and doesn't do performance any favours.
Is there any significant downside if we do track the source of hardlinks?
Cheers,
Richard
Quoted 47 lines · Richard Purdie
Quoted 26 lines · Seebs
Quoted 9 lines · Richard Purdie
No, we're actually expecting it to retain the mode it was given via the
hardlink under pseudo.This is what a real world system would do and in this case we could track it via pseudo since pseudo is loaded when the hardlink is created. It would be unreasonable for pseudo to track all hardlinks but
tracking ones created under it does seem reasonable?Hmm. Well, strictly speaking, the link created under pseudo *does* get
tracked. Hmm. But an implicit request to track also the thing linked to
is not a horrible idea. Although you'd still be able to beat it:$ touch file1 $ ln file1 file2 $ pseudo # ln file2 file3 # chmod 777 file3 # rm file2 file3 # ls -l file1Quoted 2 lines
The general case of "find everything this link also refers to" is clearly out of scope.
Agreed.
Quoted 6 lines
That said... Hmm. I think my main feeling is, if we want
to link to the file, and we want the changes to the linked file to survive, we should probably either create that file under pseudo, or explicitly claim it with pseudo when we start wanting to do the tracking.
(You can trivially do this to a tree with chown -R root tree).The trouble is that for speed, we do create trees of hardlinked files and play games with those and sstate amongst other things. Its obviously faster to do this than make physical copies of the files. Given what I know of the code paths, I suspect that tracking the source of a hardlink would make life much easier for us. Obviously we can go and start adding "chown -R" calls everywhere but that seems a little ugly to me and doesn't do performance any favours.
Quoted 2 lines
Is there any significant downside if we do track the source of hardlinks?
Would it makes sense to track the xattrs and linked files and such using some type of inode reference (virtual or otherwise)?
Since in the case of a hard link, on a normal Linux style filesystem, there will be a single inode that has a reference count higher then 1. Thus you can know the modes, xattrs, etc for that inode.. then the file points to the inode with reference counts. (this might require a rework on internal structures.. but also might solve the problem.)
--Mark
Quoted 7 lines
Cheers,
Richard
Quoted 8 lines · Mark Hatle
Would it makes sense to track the xattrs and linked files and such using some type of inode reference (virtual or otherwise)?
Since in the case of a hard link, on a normal Linux style filesystem, there will be a single inode that has a reference count higher then 1. Thus you can know the modes, xattrs, etc for that inode.. then the file points to the inode with reference counts. (this might require a rework on internal structures.. but also might solve the problem.)
Well, that *is* how we track xattrs. And everything else, we do use device and inode, but, we maintain one row for each path, and delete the individual path rows.
This gets back to one of the original design goals, which was to avoid
all the horrific things that happen to fakeroot because it's only tracking device/inode. We *want* the multiple path entries so we can report apparent database problems.
(Side note: There's some performance optimizations that have reduced stability/robustness, and I have plans involving making pseudo smarter about those, and/or allowing configuration to prefer being more cautious.)
-s
Quoted 47 lines · Richard Purdie
Quoted 26 lines · Seebs
Quoted 9 lines · Richard Purdie
No, we're actually expecting it to retain the mode it was given via the
hardlink under pseudo.This is what a real world system would do and in this case we could track it via pseudo since pseudo is loaded when the hardlink is created. It would be unreasonable for pseudo to track all hardlinks but
tracking ones created under it does seem reasonable?Hmm. Well, strictly speaking, the link created under pseudo *does* get
tracked. Hmm. But an implicit request to track also the thing linked to
is not a horrible idea. Although you'd still be able to beat it:$ touch file1 $ ln file1 file2 $ pseudo # ln file2 file3 # chmod 777 file3 # rm file2 file3 # ls -l file1Quoted 2 lines
The general case of "find everything this link also refers to" is clearly out of scope.
Agreed.
Quoted 6 lines
That said... Hmm. I think my main feeling is, if we want
to link to the file, and we want the changes to the linked file to survive, we should probably either create that file under pseudo, or explicitly claim it with pseudo when we start wanting to do the tracking.
(You can trivially do this to a tree with chown -R root tree).The trouble is that for speed, we do create trees of hardlinked files and play games with those and sstate amongst other things. Its obviously faster to do this than make physical copies of the files. Given what I know of the code paths, I suspect that tracking the source of a hardlink would make life much easier for us. Obviously we can go and start adding "chown -R" calls everywhere but that seems a little ugly to me and doesn't do performance any favours.
Quoted 2 lines
Is there any significant downside if we do track the source of hardlinks?
AFAIK, no.
And when remove file2, but file1's permission is changed, it should be considered as a bug.
// Robert
Quoted 8 lines
Cheers,
Richard
Quoted 2 lines · Robert Yang
And when remove file2, but file1's permission is changed, it should be considered as a bug.
I'm not sure of that. My interpretation would be that hard linking under pseudo to files which weren't created under the same pseudo database is user error; that's not how the database is intended to work. That said, it's pretty trivial to add the things to it.
Although I'd like to know more about the use cases for these, because it
occurs to me that the qualifier "same pseudo database" points out another
possible failure mode: Would any of those files that are being linked to
be getting linked to from *more than one* pseudo database? Because if they were, that would be a thing I haven't been planning for and I don't know whether it'd work sanely.
-s
Quoted 5 lines · Seebs
Quoted 2 lines · Robert Yang
And when remove file2, but file1's permission is changed, it should be considered as a bug.
Quoted 7 lines
I'm not sure of that. My interpretation would be that hard linking under pseudo to files which weren't created under the same pseudo database is user error; that's not how the database is intended to work. That said, it's pretty trivial to add the things to it.
Although I'd like to know more about the use cases for these, because it occurs to me that the qualifier "same pseudo database" points out another
Currently, the problem in oe-core is:
1) bitbake gzip
2) Edit rpm-native or package.bbclass to make do_package re-run.
3) bitbake gzip
After the first build, build/version.c in gzip-dbg is 0444, but after the second build, it will be 0644, this is because do_package does:
$ ln ${B}/version.c gzip-dbg/version.c,
$ chmod 0444 gzip-dbg/version.c (it runs chmod 0644 on the real filesystem) And in the second build, the gzip-dbg/version.c will be removed and created again, so that stat() can't get 0444 but 0644 since ${B}/version.c is not tracked by pseudo.
// Robert
Quoted 6 lines
possible failure mode: Would any of those files that are being linked to be getting linked to from *more than one* pseudo database? Because if they were, that would be a thing I haven't been planning for and I don't know whether it'd work sanely.
-s
Quoted 18 lines · Robert Yang
Quoted 5 lines · Seebs
Quoted 2 lines · Robert Yang
And when remove file2, but file1's permission is changed, it should be considered as a bug.
Quoted 7 lines
I'm not sure of that. My interpretation would be that hard linking under pseudo to files which weren't created under the same pseudo database is user error; that's not how the database is intended to work. That said, it's pretty trivial to add the things to it.
Although I'd like to know more about the use cases for these, because it occurs to me that the qualifier "same pseudo database" points out another
Currently, the problem in oe-core is:
Quoted 10 lines
1) bitbake gzip
2) Edit rpm-native or package.bbclass to makedo_packagere-run.
3) bitbake gzip
After the first build,build/version.cin gzip-dbg is 0444, but after the second build, it will be 0644, this is becausedo_packagedoes:
$ ln${B}/version.cgzip-dbg/version.c,
$ chmod 0444gzip-dbg/version.c(it runs chmod 0644 on the real filesystem) And in the second build, thegzip-dbg/version.cwill be removed and created again, so thatstat()can't get 0444 but 0644 since${B}/version.cis not tracked by pseudo.
And please see the first email in the thread for more details.
// Robert
Quoted 9 lines
// Robert
Quoted 6 lines
possible failure mode: Would any of those files that are being linked to be getting linked to from *more than one* pseudo database? Because if they were, that would be a thing I haven't been planning for and I don't know whether it'd work sanely.
-s
Quoted 9 lines · Robert Yang
Currently, the problem in oe-core is:
1) bitbake gzip
2) Edit rpm-native or package.bbclass to makedo_packagere-run.
3) bitbake gzip
After the first build,build/version.cin gzip-dbg is 0444, but
after
the second build, it will be 0644, this is becausedo_package
does:
Quoted 7 lines
$ ln ${B}/version.c gzip-dbg/version.c,
$ chmod 0444 gzip-dbg/version.c (it runs chmod 0644 on the real
filesystem)
And in the second build, the gzip-dbg/version.c will be removed
and
created again, so that stat() can't get 0444 but 0644 since
${B}/version.c is not tracked by pseudo.Hmm. I'm a bit confused by this. Wouldn't the second build also do a "chmod 0444" on gzip-dbg/version.c? Why is it doing that chmod the first time and not the second? If it does it the second time, the fact that the underlying file's mode changed won't matter.
But in this case... While I'm fine with modifying things to track the file linked-to, it still feels like this is a usage error. Fundamentally, we're unpacking a file (${B}/version.c), then linking to it, changing the link in some way, deleting the link, and expecting the original file to be unchanged. That doesn't seem right to me.
-s
Quoted 15 lines · Seebs
Quoted 12 lines · Robert Yang
Currently, the problem in oe-core is:
1) bitbake gzip
2) Edit rpm-native or package.bbclass to makedo_packagere-run.
3) bitbake gzip
After the first build,build/version.cin gzip-dbg is 0444, but after the second build, it will be 0644, this is becausedo_packagedoes:
$ ln${B}/version.cgzip-dbg/version.c,
$ chmod 0444gzip-dbg/version.c(it runs chmod 0644 on the real filesystem) And in the second build, thegzip-dbg/version.cwill be removed and created again, so thatstat()can't get 0444 but 0644 since${B}/version.cis not tracked by pseudo.
Quoted 2 lines
Hmm. I'm a bit confused by this. Wouldn't the second build also do a "chmod 0444" on
gzip-dbg/version.c? Why is it doing that chmod the first
Because the stat() gets 0644 on ${B}/version.c in the second run, so it would run chmod 0644 rather than 0444 on gzip-dbg/version.c. And why it gets 0644 ? pseudo's chmod automatically adds "w" on the real file, so:
if -e gzip-dbg/version.c; then
${B}/version.c = 0444
else
${B}/version.c = 0644fi
And in the second run, gzip-dbg/version.c is removed and will be recreated, so that it got 0644.
Quoted 4 lines
time and not the second? If it does it the second time, the fact that the underlying file's mode changed won't matter.
But in this case... While I'm fine with modifying things to track the
Thanks, I will send a patch for it.
Quoted 4 lines
file linked-to, it still feels like this is a usage error. Fundamentally, we're unpacking a file (
${B}/version.c), then linking to it, changing the link in some way, deleting the link, and expecting the original file to be unchanged. That doesn't seem right to me.
But that is what the real filesystem works without pseudo:
$ touch file1 $ ln file1 file2 $ chmod 777 file2 $ rm -f file2
file1 will be 777 on the real filesystem.
// Robert
Quoted 2 lines
-s
Quoted 2 lines · Robert Yang
Because the
stat()gets 0644 on${B}/version.cin the second run, so it would run chmod 0644 rather than 0444 ongzip-dbg/version.c.
Why is it calling chmod at all? The goal is apparently to give gzip-dbg/version.c the same mode that ${B}/version.c has. Since it's a hard link, no chmod call is needed at all.
Quoted 6 lines
Quoted 4 lines
time and not the second? If it does it the second time, the fact that the underlying file's mode changed won't matter.
But in this case... While I'm fine with modifying things to track the
Thanks, I will send a patch for it.
I already have a patch for this.
Quoted 5 lines
Quoted 4 lines
file linked-to, it still feels like this is a usage error. Fundamentally, we're unpacking a file (
${B}/version.c), then linking to it, changing the link in some way, deleting the link, and expecting the original file to be unchanged. That doesn't seem right to me.
Quoted 7 lines
But that is what the real filesystem works without pseudo:
$ touch file1 $ ln file1 file2 $ chmod 777 file2 $ rm -f file2file1 will be 777 on the real filesystem.
Yes. But it seems that the mode the file is being changed to is dependant on the original reported mode. And that's the part that I'm confused by; we're relying on the mode of the original file, but we're also changing
the mode of a hard link to it. This makes no sense to me.
So, I have a likely fix handy. (The difference between mine and the
version you proposed earlier is that, as I recall, yours does the LINK
operation on the original file unconditionally, which is in error; it
should only be done when no existing data was found in the database.) I'll double-check that again and go through looking for loose ends, because I have a vague feeling that I'm overlooking a thing, but I haven't figured out what it would be yet.
-s
Quoted 8 lines · Seebs
Quoted 2 lines · Robert Yang
Because the
stat()gets 0644 on${B}/version.cin the second run, so it would run chmod 0644 rather than 0444 ongzip-dbg/version.c.Why is it calling chmod at all? The goal is apparently to give
gzip-dbg/version.cthe same mode that${B}/version.chas. Since it's a hard link, no chmod call is needed at all.
It is worth trying, I will try to remove os.chown() and os.chmod() from package.bbclass to see what will happen:
if not cpath.islink(file):
os.link(file, fpath)
fstat = cpath.stat(file) os.chmod(fpath, fstat.st_mode) os.chown(fpath, fstat.st_uid, fstat.st_gid)
Quoted 23 lines
Quoted 7 lines
Quoted 5 lines
time and not the second? If it does it the second time, the fact that the underlying file's mode changed won't matter.
But in this case... While I'm fine with modifying things to track the
Thanks, I will send a patch for it.
I already have a patch for this.
Quoted 5 lines
Quoted 4 lines
file linked-to, it still feels like this is a usage error. Fundamentally, we're unpacking a file (
${B}/version.c), then linking to it, changing the link in some way, deleting the link, and expecting the original file to be unchanged. That doesn't seem right to me.Quoted 7 lines
But that is what the real filesystem works without pseudo:
$ touch file1 $ ln file1 file2 $ chmod 777 file2 $ rm -f file2file1 will be 777 on the real filesystem.
Quoted 8 lines
Yes. But it seems that the mode the file is being changed to is dependant on the original reported mode. And that's the part that I'm confused by; we're relying on the mode of the original file, but we're also changing the mode of a hard link to it. This makes no sense to me.
So, I have a likely fix handy. (The difference between mine and the version you proposed earlier is that, as I recall, yours does the LINK operation on the original file unconditionally, which is in error; it
I had checked pseudo's source code, I didn't find any function that can check this, and I appreciated that if you can fix it:-).
// Robert
Quoted 6 lines
should only be done when no existing data was found in the database.) I'll double-check that again and go through looking for loose ends, because I have a vague feeling that I'm overlooking a thing, but I haven't figured out what it would be yet.
-s
Quoted 8 lines · Robert Yang
It is worth trying, I will try to remove
os.chown()andos.chmod()from package.bbclass to see what will happen:if not
cpath.islink(file):
os.link(file, fpath)
fstat =cpath.stat(file)os.chmod(fpath, fstat.st_mode)os.chown(fpath, fstat.st_uid, fstat.st_gid)
... Okay, I admit, I can't figure this one out. Can anyone explain this?
Does python's os.link work across devices maybe?
-s
Quoted 2 lines · Seebs
Does python's os.link work across devices maybe?
The documentation at least says hard link explicitly, so presumably not.
Ross
Quoted 40 lines · Seebs
Quoted 3 lines · Robert Yang
Because the
stat()gets 0644 on${B}/version.cin the second run, so it
would run chmod 0644 rather than 0444 ongzip-dbg/version.c.Why is it calling chmod at all? The goal is apparently to give
gzip-dbg/version.cthe same mode that${B}/version.chas. Since it's a hard link, no chmod call is needed at all.Quoted 6 lines
Quoted 4 lines
time and not the second? If it does it the second time, the fact that the underlying file's mode changed won't matter.
But in this case... While I'm fine with modifying things to track the
Thanks, I will send a patch for it.
I already have a patch for this.
Quoted 7 lines
Quoted 6 lines
file linked-to, it still feels like this is a usage error. Fundamentally,
we're unpacking a file (${B}/version.c), then linking to it, changing the link in some way, deleting the link, and expecting the original file
to be unchanged. That doesn't seem right to me.Quoted 7 lines
But that is what the real filesystem works without pseudo:
$ touch file1 $ ln file1 file2 $ chmod 777 file2 $ rm -f file2file1 will be 777 on the real filesystem.
Yes. But it seems that the mode the file is being changed to is dependant on the original reported mode. And that's the part that I'm confused by;
we're relying on the mode of the original file, but we're also changing the mode of a hard link to it. This makes no sense to me.
Quoted 8 lines
So, I have a likely fix handy. (The difference between mine and the version you proposed earlier is that, as I recall, yours does the LINK operation on the original file unconditionally, which is in error; it should only be done when no existing data was found in the database.) I'll double-check that again and go through looking for loose ends, because
I have a vague feeling that I'm overlooking a thing, but I haven't figured out what it would be yet.
A better way to describe the problem:
I create a file:
$ touch my-special-file $ chmod 0444 my-special-file
I then run pseudo
$ pseudo ln my-special-file file1 $ stat file1 Access: (0444/-r--r--r--) Uid: ( 0/ root) Gid: ( 0/ root) $ pseudo rm -f file1 $ # remove pseudo database
At this point the original 'my-special-file' is now 0644, not 0444. This is because pseudo changes the perms on the file so that it could pretend to be root.
So if we repeat the last steps:
$ pseudo ln my-special-file file1 $ stat file1 Access: (0644/-r--r--r--) Uid: ( 0/ root) Gid: ( 0/ root) $ pseudo rm -f file1 $ # remove pseudo database
So the problem is -- pseudo is modifying the 'original' file perms, which on a new instance of the pseudo database then gets inherited. So we get unpredictable results if this is the first time through -- or not the first time through.
The problem is 'my-special-file' is never owned by pseudo -- but that isn't really what the problem is -- the problem is because it's a hardlink -- and pseudo is changing perms to emulate root -- it triggers a QA fault because the file is 'changing'.
--Mark
Quoted 2 lines
-s
Quoted 4 lines · Mark Hatle
So the problem is -- pseudo is modifying the 'original' file perms, which on a new instance of the pseudo database then gets inherited. So we get unpredictable results if this is the first time through -- or not the first time through.
Yeah. And the problem here is in part that we're calling chmod on a file linked to a file external to pseudo, but we're also checking that file's mode to find out what permissions we want it to have. Which is probably an error.
So, it does look like we need the tracking, but also if we stopped doing:
stat file1 # obtain mode
ln file1 file2
chmod <mode> file2
we'd save significant time AND avoid the problem.
-s
Quoted 17 lines · Seebs
Quoted 6 lines · Mark Hatle
So the problem is -- pseudo is modifying the 'original' file perms, which on a
new instance of the pseudo database then gets inherited. So we get unpredictable results if this is the first time through -- or not the first time
through.Yeah. And the problem here is in part that we're calling chmod on a file linked to a file external to pseudo, but we're also checking that file's mode to find out what permissions we want it to have. Which is probably an error.
So, it does look like we need the tracking, but also if we stopped doing:
Quoted 8 lines
stat file1 # obtain mode
ln file1 file2
chmod <mode> file2we'd save significant time AND avoid the problem.
-s
The alternative to the 'ln' is a 'cp' operation. This is how it used to work until optimizations were added a few releases ago. It was observed that this saves a large amount of space -- but it had the unintended consequence of suddenly files are now changes as the do_install/do_package processing manipulates files and modes change...
I almost wonder if in pseudo we should avoid changing any modes internal to the file -- until something tries to write to the file. (Can we even capture that and try to fix it? If not, what we're doing is likely the only answer.)
--Mark
Quoted 5 lines · Mark Hatle
The alternative to the 'ln' is a 'cp' operation. This is how it used to work until optimizations were added a few releases ago. It was observed that this saves a large amount of space -- but it had the unintended consequence of suddenly files are now changes as the do_install/do_package processing manipulates files and modes change...
Yeah, and I think if we just dropped the copy of permissions, this would at least temporarily go away.
Quoted 3 lines
I almost wonder if in pseudo we should avoid changing any modes internal to the file -- until something tries to write to the file. (Can we even capture that and try to fix it? If not, what we're doing is likely the only answer.)
I don't think we usefully can.
Hmm.
So it occurs to me, there's only a few cases where we actually have any
reason to make on-filesystem permissions changes at all. Say, execute bits for executables. Otherwise, we could in theory just not bother writing changes to the disk, because the disk permissions are irrelevant.
-s
Quoted 2 lines · Richard Purdie
The trouble is that for speed, we do create trees of hardlinked files and play games with those and sstate amongst other things.
Yeah. One concern would be ownership: If the things we're hardlinking to
weren't created/tracked by pseudo initially, they're going to show up as owned by the actual user doing the build. So hardlinks to them will also.
So it may be more reasonable to just create those things under pseudo also, in which case they're already tracked.
Quoted 9 lines
Its
obviously faster to do this than make physical copies of the files. Given what I know of the code paths, I suspect that tracking the source of a hardlink would make life much easier for us. Obviously we can go and start adding "chown -R" calls everywhere but that seems a little ugly to me and doesn't do performance any favours.Is there any significant downside if we do track the source of hardlinks?
Hmm. I'm not sure. It'll make things more complicated, but only marginally. The net impact would be fewer surprises in common cases, but the uncommon cases would get *really* weird. But it seems reasonable. (At least for the case where there isn't an existing entry.) I'll
poke at this a bit, I think.
-s