Re: old->umask copying without spin_lock, in copy_fs_struct()

[Al Viro <viro@ZenIV.linux.org.uk>]

On Sun, Apr 07, 2013 at 11:37:27AM +0600, Rakib Mullick wrote:
> Hello,
> 
> In copy_fs_struct(), old->umask is assigned to fs->umask outside of
> spin_lock(&old->lock). Shouldn't it be inside spin_lock()? Since we're
> dealing with  fs_struct *old ? Isn't it unsafe? Following lines -
> 
>               fs->umask = old->umask;
> 
>                 spin_lock(&old->lock);

What would moving it down buy us?  Root, pwd and umask are all modified
independently; the *only* reason why we hold old->lock for root and
pwd (and we might drop and regain it between copying those - it would
be pointless, so we don't bother, but it wouldn't have affected correctness)
is that we want the values of root.mnt and root.dentry taken at the same
time and we want to grab extra references on those while they are still
valid.  The same goes for pwd, of course.  That's what old->lock
protects - we want the damn thing atomic wrt set_fs_root() and set_fs_pwd().
umask is an integer; its updates are atomic anyway, so it's not as if we
could see a half-updated value or needed to do anything with refcounts.