Re: [PATCH 1/2] kernel/sys.c: return the current gid when error occurs

[Chen Gang <gang.chen@asianux.com>]

On 08/08/2013 12:58 AM, Andy Lutomirski wrote:
> On Wed, Aug 7, 2013 at 9:21 AM, Oleg Nesterov <oleg@redhat.com> wrote:
>> On 08/06, Andy Lutomirski wrote:
>>>
>>> I assume that what the man page means is that the return value is
>>> whatever fsgid was prior to the call.  On error, fsgid isn't changed, so
>>> the return value is still "current".
>>
>> Probably... Still
>>
>>         On success, the previous value of fsuid is returned.
>>         On error, the current value of fsuid is returned.
>>
>> looks confusing. sys_setfsuid() always returns the old value.
>>
>>> (FWIW, this behavior is awful and is probably the cause of a security
>>> bug or three, since success and failure are indistinguishable.
>>
>> At least this all looks strange.
>>
>> I dunno if we can change this old behaviour. I won't be surprized
>> if someone already uses setfsuid(-1) as getfsuid().
> 

Oh, really it is.

Hmm... as a pair function, we need add getfsuid() too, if we do not add
it, it will make negative effect with setfsuid().

Since it is a system call, we have to keep compitable.

So in my opinion, better add getfsuid2()/setfsuid2() instead of current
setfsuid()

  for getfsuid2() can just reference to getuid() definition.
  for setfsuid2() can just reference to setuid() definition.
    (which can return error code when failure occurs).

If possible, I will/should sent the related patches for it.

> I suspect that changing this without introducing security or other
> bugs is impossible.  If someone wanted to add new_setfsuid that
> returned an error when it failed, that would be a different story.
> (I'm not going to do that myself.)
> 
>>
>> And perhaps the man page should be changed. Add Michael.
> 
> Agreed.  The text is a bit odd.
> 

I agree that the text is odd, since it is an system call API, we have
to change it to match our current kernel implementation which is:

  "for system call, whether succeed or not, it always return the previous value, the caller can not know whether succeed or not directly"
  "if the caller need know about success or not, the demo like below"
  "	setfsuid(new);"
  "	if (setfsuid(-1) != new)"
  "		/* report failure */" 

And better to give an additional comment:

  "currently, new system call getfsuid2()/setfsuid2() are supplied, setfsuid() is obseleted"

>>
>> Oleg.
>>
> 
> 
> 

Thanks.
-- 
Chen Gang