linux-kernel - Re: [patch 05/11] syslets: core code

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:	Thu, 15 Feb 2007 10:25:37 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Evgeniy Polyakov <johnpol@....mipt.ru>
cc:	Ingo Molnar <mingo@...e.hu>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Arjan van de Ven <arjan@...radead.org>,
	Christoph Hellwig <hch@...radead.org>,
	Andrew Morton <akpm@....com.au>,
	Alan Cox <alan@...rguk.ukuu.org.uk>,
	Ulrich Drepper <drepper@...hat.com>,
	Zach Brown <zach.brown@...cle.com>,
	"David S. Miller" <davem@...emloft.net>,
	Benjamin LaHaise <bcrl@...ck.org>,
	Suparna Bhattacharya <suparna@...ibm.com>,
	Davide Libenzi <davidel@...ilserver.org>,
	Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [patch 05/11] syslets: core code

On Thu, 15 Feb 2007, Evgeniy Polyakov wrote:
> 
> So we just need to describe the way we want to see new interface -
> that's it.

Agreed. Absolutely.

But please keep the kernel interface as part of that. Not just a strange 
and complex kernel interface and then _usable_ library interfaces that use 
the strange and complex one internally. Because if the complex one has no 
validity on its own, it's just (a) a bitch to debug and (b) if we ever 
change any details inside the kernel we'll end up with a lot of subtle 
code where user land creates complex data, and the kernel just reads it, 
and both just (unnecessarily) work around the fact that the other doesn't 
do the straightforward thing.

> Here is a stub for async_stat() - probably broken example, but that does
> not matter - this interface is really easy to change.
> 
> static void syslet_setup(struct syslet *s, int nr, void *arg1...)
> {
> 	s->flags = ...
> 	s->arg[1] = arg1;
> 	....
> }
> 
> long glibc_async_stat(const char *path, struct stat *buf)
> {
> 	/* What about making syslet and/or set of atoms per thread and preallocate 
> 	 * them when working threads are allocated? */
> 	struct syslet s;
> 	syslet_setup(&s, __NR_stat, path, buf, NULL, NULL, NULL, NULL);
> 	return async_submit(&s);
> }

And this is a classic example of potentially totally buggy code.

Why? You're releasing the automatic variable on the stack before it's 
necessarily all used!

So now you need to do a _longterm_ allocation, and that in turn means that 
you need to do a long-term de-allocation!

Ok, so do we make the rule be that all atoms *have* to be read fully 
before we start the async submission (so that the caller doesn't need to 
do a long-term allocation)?

Or do we make the rule be that just the *first* atom is copied by the 
kernel before the async_sumbit() returns, and thus it's ok to do the above 
*IFF* you only have a single system call?

See? The example you tried to use to show how "simple" the interface iswas 
actually EXACTLY THE REVERSE. It shows how subtle bugs can creep in!

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