linux-kernel - Re: [patch 10/13] signal/timer/event fds v8

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:	Fri, 30 Mar 2007 18:26:03 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Davide Libenzi <davidel@...ilserver.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Ingo Molnar <mingo@...e.hu>,
	Suparna Bhattacharya <suparna@...ibm.com>,
	Zach Brown <zach.brown@...cle.com>,
	Benjamin LaHaise <bcrl@...ck.org>
Subject: Re: [patch 10/13] signal/timer/event fds v8 - eventfd core ...

On Fri, 30 Mar 2007 18:11:55 -0700 (PDT) Davide Libenzi <davidel@...ilserver.org> wrote:

> 
> > > + */
> > 
> > So it is the caller's responsibility to ensure that *file refers to an
> > eventfd file?
> 
> In which function? I lost you ...
> 

eventfd_signal() assumes that the passed in file* refers to an eventfd
file.  So if a caller passes in a file* for /etc/passwd, the kernel will go
splat.

I guess that's caveat emptor, and any violations of that will show up
quickly in testing.  My main concern would be that there might be some way
for a naughty user to force the kernel to pass a non-eventfd file* into
this function.  That depends upon as-yet-unwritten code - is there a risk
of this happening, and how do we prevent it?

> 
> > > +int eventfd_signal(struct file *file, int n)
> > > +{
> > > +	struct eventfd_ctx *ctx = file->private_data;
> > > +	unsigned long flags;
> > > +
> > > +	if (n < 0)
> > > +		return -EINVAL;
> > > +	spin_lock_irqsave(&ctx->lock, flags);
> > > +	if (ULLONG_MAX - ctx->count < n)
> > > +		n = (int) (ULLONG_MAX - ctx->count);
> > > +	ctx->count += n;
> > > +	if (waitqueue_active(&ctx->wqh))
> > > +		wake_up_locked(&ctx->wqh);
> > > +	spin_unlock_irqrestore(&ctx->lock, flags);
> > > +
> > > +	return n;
> > > +}
>
>
>
> > > +	DECLARE_WAITQUEUE(wait, current);
> > > +
> > > +	if (count < sizeof(ucnt))
> > > +		return -EINVAL;
> > > +	if (get_user(ucnt, (const __u64 __user *) buf))
> > > +		return -EFAULT;
> > 
> > Some architectures do not implement 64-bit get_user()
> 
> copy_from_user it is, then ...
> 

spose so.  I think architectures _should_ implement 64-bit get_user() and
put_user() nowadays.  So you could leave the code as-is and inform the arch
maintainers, if you're feeling keen.

If all this code has its own Kconfig options then the architectures won't
break until their maintainers come along to enable the new features, so
they'll implement 64-bit get_user() at that time and things will all unfold
in a nicely non-chaotic fashion.

-
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/