linux-kernel - Re: [PATCH 26/76] tty: Add a kref count

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 6 Oct 2008 12:20:37 +0200
From:	Louis Rilling <Louis.Rilling@...labs.com>
To:	Alan Cox <alan@...hat.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 26/76] tty: Add a kref count

On Sun, Oct 05, 2008 at 05:08:15PM +0100, Alan Cox wrote:
> Introduce a kref to the tty structure and use it to protect the tty->signal
> tty references. For now we don't introduce it for anything else.

[...]

> diff --git a/kernel/fork.c b/kernel/fork.c
> index 7ce2ebe..30de644 100644
> --- a/kernel/fork.c
> +++ b/kernel/fork.c
> @@ -802,6 +802,7 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
>  
>  	sig->leader = 0;	/* session leadership doesn't inherit */
>  	sig->tty_old_pgrp = NULL;
> +	sig->tty = NULL;
>  
>  	sig->utime = sig->stime = sig->cutime = sig->cstime = cputime_zero;
>  	sig->gtime = cputime_zero;
> @@ -838,6 +839,7 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk)
>  void __cleanup_signal(struct signal_struct *sig)
>  {
>  	exit_thread_group_keys(sig);
> +	tty_kref_put(sig->tty);
>  	kmem_cache_free(signal_cachep, sig);
>  }
>  
> @@ -1227,7 +1229,8 @@ static struct task_struct *copy_process(unsigned long clone_flags,
>  				p->nsproxy->pid_ns->child_reaper = p;
>  
>  			p->signal->leader_pid = pid;
> -			p->signal->tty = current->signal->tty;
> +			tty_kref_put(p->signal->tty);
> +			p->signal->tty = tty_kref_get(current->signal->tty);
>  			set_task_pgrp(p, task_pgrp_nr(current));
>  			set_task_session(p, task_session_nr(current));
>  			attach_pid(p, PIDTYPE_PGID, task_pgrp(current));

I'm a bit puzzled by this 'tty_kref_put(p->signal->tty)'. AFAICS, this is a noop
since we are not in the CLONE_THREAD case and hence p->signal->tty == NULL.
So, is it to make the code look more consistent? If so, is it worth adding extra
code and cycles for this (I doubt that gcc is able to optimize this away)? What
kind of future changes should this code protect against?

Thanks,

Louis

-- 
Dr Louis Rilling			Kerlabs
Skype: louis.rilling			Batiment Germanium
Phone: (+33|0) 6 80 89 08 23		80 avenue des Buttes de Coesmes
http://www.kerlabs.com/			35700 Rennes

Download attachment "signature.asc" of type "application/pgp-signature" (190 bytes)