On 05/17/12 21:27, James Cammarata wrote:
On Thu, May 17, 2012 at 3:25 AM, Jun'ichi Nomura
<j-nomura(a)ce.jp.nec.com> wrote:
> while using cobbler, I've hit sometimes a strange problem,
> that cobblerd starts creating files with wrong permissions like:
>
> --w------- 2 root root 11405684 May 16 14:57 initrd.img
>
> Basically, regular files become "--w-------" and directories become
> "d-w---x---".
>
> Once happen, it continues to generate files with such permissions
> until restarting the daemon.
>
> Investigating with 'crash' command, I found cobblerd had wrong umask
> when the problem happened:
>
> crash> ps cobblerd
> PID PPID CPU TASK ST %MEM VSZ RSS COMM
> 4693 1 0 ffff810027fbc100 IN 0.7 375980 31688 cobblerd
> crash> struct task_struct.fs 0xffff810027fbc100
> fs = 0xffff810035f6fb00,
> crash> struct fs_struct.umask 0xffff810035f6fb00
> umask = 0x177,
>
> Normally it has the following umask:
>
> crash> struct fs_struct.umask 0xffff810035f6fb80
> umask = 0x12,
>
> The wrong umask explains why the generated files/directories have
> such a strange permissions.
>
> I found the following code in api.py and serializer.py are suspicious:
>
> old = os.umask(0x777)
> fd = open("/var/lib/cobbler/.mtime","w")
> fd.write("%f" % time.time())
> fd.close()
> os.umask(old)
>
> Firstly, "0x777" seems strange. I guess it should be octal
"777".
> It seems python ignores upper bits and set "0x177" when you do
> os.umask(0x777).
>
> Secondly, if the above code is not serialized each other,
> it would end up with wrong umask as follows:
>
> Context A Context B
> ------------------------------------------
> old = umask(0x777)
> old = umask(0x777)
> [old is 0x777]
> umask(old)
> umask(old)
>
> Any comments?
Very interesting, this was reported in issue #117
(
https://github.com/cobbler/cobbler/issues/117) though it hadn't gone
anywhere. I had originally thought cobbler was perhaps chmod'ing files
but did not look at it potentially changing umask like this. Overall,
it looks like it's being done wrong consistently through the code:
cobbler/cobblerd.py: um = os.umask(int('0027',16))
cobbler/cobblerd.py: os.umask(um)
cobbler/api.py: old = os.umask(0x777)
cobbler/api.py: os.umask(old)
cobbler/serializer.py: old = os.umask(0x777)
cobbler/serializer.py: os.umask(old)
scripts/cobblerd: os.umask(022)
That first one should probably be int('0027',8) and the last one
should probably be 0022 (that may be ok as-is).
Would it be possible for you to fork the code via github and correct
the above issues and test this? I have been unable to replicate the
issue locally so if you can reliably reproduce this you'll be better
able to test this out.
Thank you for the comments.
Unfortunately, I've seen it only a few times in a year...
I wonder whether those functions using umask are called
from threads (or signal handlers).
If they do, it would certainly cause such a problem
in the same way as the following reproducer.
---------------------------------------------------------
#!/usr/bin/python
import os
import time
import thread
import sys
origmask = 0002
badmask = 0x777
def loop(file,mask):
while True:
old = os.umask(mask)
if (old != origmask):
print "wrong umask %o vs expected %o" % (old,origmask)
sys.exit(1)
fd = open(file,"w")
fd.write("%f" % time.time())
fd.close()
os.umask(old)
os.unlink(file)
os.umask(origmask)
thread.start_new_thread(loop, ("test1", badmask))
loop("test2",origmask)
---------------------------------------------------------
And I think the solution would be something like the attached patch,
using os.open() with mode parameter to avoid os.umask.
Thanks,
--
Jun'ichi Nomura, NEC Corporation