linux-kernel - Re: [PATCH 2/2] taskstats: add_del_listener() should ignore !valid listener's

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]

Message-ID: <20110720210146.GA6273@redhat.com>
Date:	Wed, 20 Jul 2011 23:01:46 +0200
From:	Oleg Nesterov <oleg@...hat.com>
To:	Vasiliy Kulikov <segoon@...nwall.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Balbir Singh <bsingharora@...il.com>,
	Jerome Marchand <jmarchan@...hat.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] taskstats: add_del_listener() should ignore !valid
	listener's

On 07/20, Vasiliy Kulikov wrote:
>
> On Wed, Jul 20, 2011 at 21:00 +0200, Oleg Nesterov wrote:
> > When send_cpu_listeners() finds the orphaned listener it marks
> > it as !valid and drops listeners->sem. Before it takes this sem
> > for wrinting, s->pid can be reused and add_del_listener() can
> > wrongly try to re-use this entry.
> >
> > Change add_del_listener() to check ->valid = T.
>
> I think the current logic is wrong.

me too ;)

still I think this patch makes sense anyway.

> If s was claimed is invalid, it
> then points to a dead task,

I forgot everything I knew about netlink. But, it seems, netlink_bind()
can use nl_pid != current->pid. But even if this is not true,

> it is unknown when it died.  As the same pid
> can be reused by a new process BEFORE add_del_listener(), it is unknown
> whether ->pid points to the actual owner task.

And? Why should we try to reuse the !valid entries? struct listener
is just the "addr" we should send the message to. If someone calls
add_listener() it should works regardless of dead entries with the
same ->pid.

> Rather than optimizing the wrong algorithm it's better to change the
> processes keeping way.

Not sure I understand... This is not optimizing, the patch tries to
fix the unlikely (in fact mostly theoretical) and minor problem. Still
this is the problem, no?

Thanks,

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/