[linux-cifs-client] Reset of read-only attribute on Windows not reflected in client

[Alan Tyson]

I've been seeing the same problem that was mentioned in this posting
 http://lists.samba.org/archive/samba/2006-September/125279.html
and can still duplicate this problem using the 1.45 tarball for RHEL4.  
Having looked at the problem a little more, I do not believe that this 
as been resolved in the lastest git archives.

Win 2003 or Win XP server and RHEL4 client using the following mount command
 # mount -t cifs -o user=me,dom=mysom,uid=500,gid=500,file_mod=0777 \
 > //server/test /mnt

Initially, the file is writeable, so the permissions appear as 
-rwxrwxrwx.  When the read-only flag is set on the server, then the 
permissions change to -r-xr-xr-x.  However, when the read-only flag is 
reset on the server, the unix permissions remain at -r-xr-xr-x instead 
of allowing write access to the file.

Network tracing shows that the server is definitely sending back file 
attributes of 0x20 after the read-only flag has been reset (the 
read-only flag is the least sig bit of the attributes).  I can also see 
that the cached inode for the file has cifsAttrs =0x20 whereas i_mode is 
set to 0100555.

Looking at fill_in_inode I can see that the resetting of the w bits is 
easy when the read-only attribute flag is set.  All that needs to be 
done is to reset all three w bits:

201                 if (attr & ATTR_READONLY)
202                         tmp_inode->i_mode &= ~(S_IWUGO);

However, there is no else clause to this statement, so the w bits don't 
get set again if the read-only permissions are unset on the server.

Is this intentional?  If not, I would like to begin a discussion on how 
we could correct this.

I see that the main problem here is knowing which w bits to set.  If I 
add an else to the above which ORs the mode with (S_IWUGO & 
cifs_sb->mnt_file_mode) then I break chmod.  Chmod 700 ends up being 
722.  If I just set the owner's w bit, then the permissions are not 
being set back to what they were originally.  What is the feeling about 
this additional else clause?  Would this break anything?

    if (attr & ATTR_READONLY)
            tmp_inode->i_mode &= ~(S_IWUGO);
    else {
            /* if no w bits are set, assume that the flag has been changed
               on the server and set the same w bits as would be set at
               mount time
             */
            if ((tmp_inode->i_mode & S_IWUGO) == 0)
                    tmp_inode->i_mode |= (S_IWUGO & cifs_sb->mnt_file_mode);
    }

- Alan Tyson, HP Services, UK.