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] [day] [month] [year] [list]
Message-ID: <20151222010403.GX20997@ZenIV.linux.org.uk>
Date:	Tue, 22 Dec 2015 01:04:04 +0000
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC] free_pages stuff

On Mon, Dec 21, 2015 at 04:03:11PM -0800, Linus Torvalds wrote:

> If you want to have versions of the function that return pointers, you
> had damn well better give them new names. Not use the same name for a
> different function, causing confusion and forcing this kind of crazy
> "change everything at once" flag-day patches, and pain for
> backporting.

*shrug*

Up to you.  I'll cherry-pick the fixes for bugs found in process and leave
the rest alone.
 
> And quite frankly, even the "new name" is likely a bad idea. If you
> want to allocate a page, and get a pointer, just use "kmalloc()".
> Boom, done!

Erm...  You've just described the absolute majority of callers.  Really.
Counting typecasts demonstrates _that_ very clearly.  Any place that
wanted to allocate a page and get a pointer needs a typecast in mainline;
any place that wanted to allocate a page and get a number would need
one after this series.  Similar for freeing something that is a pointer
and is a number respectively.

The total after that series was 70 typecasts added and 1408 removed.  In other
words, "want to allocate a page and get a pointer" outnumbers the "want to
allocate a page and get a number" a _lot_.

Do you really mean that we are overusing __get_free_page() and friends that
much and that we should simply use kmalloc() instead?  I'm not saying that
it's wrong - a lot of places clearly would be fine with kmalloc/kfree.
For something like page table allocations kmalloc() is obviously wrong (and
they also are of "get a pointer" sort), but that's a very small fraction.

> So I don't know how many ways I can say "NO", but I'll not take
> anythign like this. It's *completely* wrong.

OK.  Don't get me wrong - I'm not fond of all-over-the-tree changes either;
I wanted to figure out how the damn thing is actually used and I have found
that.  What (if anything) to do with that is a separate question.

For me the bottom line so far is that we have a lot of places where page
allocator is used and the majority of those uses the result as a pointer.
That, with the calling conventions we have (and had all along), means tons
of boilerplate.  It also means a lot of opportunities to mix physical,
virtual and DMA addresses, since typechecking is completely bypassed by
those typecasts.

If I understood you correctly, in a lot of those cases the answer should've
been "just use kmalloc() and be done with that".  And something like
e.g. debugfs read and write methods of some wireless NIC driver
certainly could use kmalloc(); any concerns about extra overhead are
ridiculous there.  The same goes for e.g. sysfs symlink body generated
when we run into one, etc.  I'm not suggesting to start converting existing
code to kmalloc; that only goes for new code being written, obviously.

PS: in case you've said that "NO" earlier and I'd missed your replies, my
apologies for keeping that up; this is really the first response from you I've
seen on this topic.
--
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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ