2009-06-16 20:12:57

by Oleg Nesterov

[permalink] [raw]
Subject: [PATCH] shift current_cred() from __f_setown() to f_modown()

Shift current_cred() from __f_setown() to f_modown(). This reduces
the number of arguments and saves 48 bytes from fs/fcntl.o.

Signed-off-by: Oleg Nesterov <[email protected]>

--- a/fs/fcntl.c~f_modown 2009-04-06 00:03:41.000000000 +0200
+++ b/fs/fcntl.c 2009-06-16 21:41:18.000000000 +0200
@@ -196,15 +196,19 @@ static int setfl(int fd, struct file * f
}

static void f_modown(struct file *filp, struct pid *pid, enum pid_type type,
- uid_t uid, uid_t euid, int force)
+ int force)
{
write_lock_irq(&filp->f_owner.lock);
if (force || !filp->f_owner.pid) {
put_pid(filp->f_owner.pid);
filp->f_owner.pid = get_pid(pid);
filp->f_owner.pid_type = type;
- filp->f_owner.uid = uid;
- filp->f_owner.euid = euid;
+
+ if (pid) {
+ const struct cred *cred = current_cred();
+ filp->f_owner.uid = cred->uid;
+ filp->f_owner.euid = cred->euid;
+ }
}
write_unlock_irq(&filp->f_owner.lock);
}
@@ -212,14 +216,13 @@ static void f_modown(struct file *filp,
int __f_setown(struct file *filp, struct pid *pid, enum pid_type type,
int force)
{
- const struct cred *cred = current_cred();
int err;
-
+
err = security_file_set_fowner(filp);
if (err)
return err;

- f_modown(filp, pid, type, cred->uid, cred->euid, force);
+ f_modown(filp, pid, type, force);
return 0;
}
EXPORT_SYMBOL(__f_setown);
@@ -245,7 +248,7 @@ EXPORT_SYMBOL(f_setown);

void f_delown(struct file *filp)
{
- f_modown(filp, NULL, PIDTYPE_PID, 0, 0, 1);
+ f_modown(filp, NULL, PIDTYPE_PID, 1);
}

pid_t f_getown(struct file *filp)


2009-06-16 20:21:05

by David Howells

[permalink] [raw]
Subject: Re: [PATCH] shift current_cred() from __f_setown() to f_modown()

Oleg Nesterov <[email protected]> wrote:

> Shift current_cred() from __f_setown() to f_modown(). This reduces
> the number of arguments and saves 48 bytes from fs/fcntl.o.
>
> Signed-off-by: Oleg Nesterov <[email protected]>

Acked-by: David Howells <[email protected]>

2009-06-16 20:22:47

by Linus Torvalds

[permalink] [raw]
Subject: Re: [PATCH] shift current_cred() from __f_setown() to f_modown()



On Tue, 16 Jun 2009, Oleg Nesterov wrote:
>
> Shift current_cred() from __f_setown() to f_modown(). This reduces
> the number of arguments and saves 48 bytes from fs/fcntl.o.

Ok, I know I asked for this, but now I suddenly start worrying about
whether f_owner.uid/euid are initialized at all for the pid==NULL case?

They used to be initialized to zero, now they are left alone. Are they
initialized somewhere else earlier?

Linus

2009-06-16 21:05:49

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] shift current_cred() from __f_setown() to f_modown()

On 06/16, Linus Torvalds wrote:
>
>
> On Tue, 16 Jun 2009, Oleg Nesterov wrote:
> >
> > Shift current_cred() from __f_setown() to f_modown(). This reduces
> > the number of arguments and saves 48 bytes from fs/fcntl.o.
>
> Ok, I know I asked for this, but now I suddenly start worrying about
> whether f_owner.uid/euid are initialized at all for the pid==NULL case?
>
> They used to be initialized to zero, now they are left alone. Are they
> initialized somewhere else earlier?

I think this is fine, but I should have mentioned this in the changelog.

If f_owner.pid == NULL we never use f_owner.uid/euid. Otherwise we have
a bug anyway: we must not send signals if pid was reset to NULL.

Oleg.

2009-06-16 22:57:36

by James Morris

[permalink] [raw]
Subject: Re: [PATCH] shift current_cred() from __f_setown() to f_modown()

On Tue, 16 Jun 2009, Oleg Nesterov wrote:

> Shift current_cred() from __f_setown() to f_modown(). This reduces
> the number of arguments and saves 48 bytes from fs/fcntl.o.
>
> Signed-off-by: Oleg Nesterov <[email protected]>

Reviewed-by: James Morris <[email protected]>


--
James Morris
<[email protected]>