[PATCH] cifs: Support for an upcall to map SID to an uid and a gid

[Shirish Pargaonkar]

On Mon, Dec 13, 2010 at 5:16 AM, Jeff Layton <jlayton at samba.org> wrote:
> On Mon, 13 Dec 2010 14:22:09 +1100
> Andrew Bartlett <abartlet at samba.org> wrote:
>
>> On Sun, 2010-12-12 at 06:39 -0500, Jeff Layton wrote:
>> > On Sun, 12 Dec 2010 14:48:04 +1100
>> > Andrew Bartlett <abartlet at samba.org> wrote:
>> >
>> > > On Sat, 2010-12-11 at 22:11 -0500, Jeff Layton wrote:
>> > > > On Sat, 11 Dec 2010 19:57:11 -0500
>> > > > Richard Sharpe <realrichardsharpe at gmail.com> wrote:
>> > > >
>> > > > > On Sat, Dec 11, 2010 at 7:30 PM, Jeff Layton <jlayton at samba.org> wrote:
>> > > > > >>
>> > > > > >> Will look into this.  One thing that concerns me is if a cached etnry
>> > > > > >> for a SID with its name and an id (either an uid or a gid), if that SID
>> > > > > >> now represents a different object and has differernt name, would
>> > > > > >> not cached info be incorrect?  Not sure if this can ever happen
>> > > > > >> or how would it happen and if it does, what would be a trigger
>> > > > > >> for a cache revalidation and purges!
>> > > > > >>
>> > > > > >
>> > > > > > Sure, mappings can change. But, you still have the same problem with
>> > > > > > what you're proposing in these patches. The userspace program isn't
>> > > > > > setting a timeout on the key. Once a mapping is put in the keyring,
>> > > > > > it's there until it's revoked. You probably want to set a max TTL for
>> > > > > > the entries in the cache regardless of what scheme is used.
>> > > > >
>> > > > > I was under the impression that SIDs are never reused. Perhaps I am mistaken.
>> > > > >
>> > > >
>> > > > That may be, but the mapping of a SID is dependent upon settings in
>> > > > config files that could change. It seems reasonable to me to only cache
>> > > > these mappings for a period of time in the event that they do. That
>> > > > period of time could default to being rather long and be tunable.
>> > >
>> > > I think that instead some explicit signal should be made to indicate
>> > > that a mapping has changed, so you don't have to worry about cache
>> > > times.  It should change *very* rarely and only on specific
>> > > administrator intervention.  We do a lot of things to avoid this
>> > > happening in the normal course of events.
>> > >
>> >
>> > What would provide this signal? winbindd? I suppose we could add a knob
>> > or something under /sys that tells cifs to dump the idmap cache.
>>
>> I think a /sys knob seems appropriate, perhaps easily sent a command
>> option on the same utility used for the upcall?
>>
>> > We would also have to consider however how to deal with someone running
>> > an old winbindd that doesn't signal the kernel properly.
>>
>> That's a very interesting question, as after a manual reconfiguration
>> perhaps even winbind might not know it changed.  It depends how deeply
>> the administrator changed things (changing the idmap_rid config settings
>> might matter for example).  I'll let others who deal with idmap more
>> often comment.
>>
>
> The other option is just to have a manual knob that flushes the cache,
> and add something like this to the cifs.upcall manpage: "If you change
> your idmapping configuration, then you'll probably also want to flush
> the idmap cache." Maybe it's a rare enough thing that we shouldn't
> sweat trying to make it too automatic.
>
> --
> Jeff Layton <jlayton at samba.org>
>

# date; cat /proc/keys | grep cifs
Tue Dec 14 16:16:20 CST 2010
26fb212d I-----     1 perm 1f030000     0     0 keyring   .cifs_idmap: empty
2fb33fb5 IR----     2 expd 1f030000     0     0 keyring   .cifs_idmap: empty

# date; cat /proc/keys | grep cifs
Tue Dec 14 16:16:29 CST 2010
26fb212d I-----     1 perm 1f030000     0     0 keyring   .cifs_idmap: 2/4
2fb33fb5 IR----     2 expd 1f030000     0     0 keyring   .cifs_idmap: empty

# date; cat /proc/keys | grep cifs
Tue Dec 14 16:20:10 CST 2010
26fb212d I-----     1 perm 1f030000     0     0 keyring   .cifs_idmap: 2/4
2fb33fb5 IR----     2 expd 1f030000     0     0 keyring   .cifs_idmap: empty


Should not these two entries expire after a minute if

key->expiry = KEY_EXPIRY_TIMEOUT;

in function cifs_idmap_key_instantiate()

if

#define KEY_EXPIRY_TIMEOUT 60*HZ

I was expecting this to be empty instead of 2/4 after more than a minute
of inactivity.

26fb212d I-----     1 perm 1f030000     0     0 keyring   .cifs_idmap: 2/4