From: computersforpeace@gmail.com (Brian Norris)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] mtd: avoid stack overflow in MTD CFI code
Date: Sun, 3 Apr 2016 23:51:34 -0700 [thread overview]
Message-ID: <20160404065134.GD13995@localhost> (raw)
In-Reply-To: <10357051.EVa2c1MaZl@wuerfel>
On Fri, Mar 18, 2016 at 09:25:23PM +0100, Arnd Bergmann wrote:
> On Friday 18 March 2016 10:44:19 Brian Norris wrote:
> > FWIW, I took a little look at this, and I can reproduce this myself. I
> > can get a large frame size on at least 2 of the 3 functions you report.
> > I think most of the gain you get with this patch is due to the Kconfig
> > change (MTD_COMPLEX_MAPPINGS) because it forces an extra level of
> > indirection. The other changes seem to give a modest decrease in size,
> > though it's less clear why exactly.
> >
> > Anyway, I think the problem isn't primarily with anything you're
> > touching here exactly, but with the fact that we're putting several
> > copies of the 'map_word' typedef on the stack, and doing assignment to
> > it as if it's a typical variable. But with MAP_WIDTH_32, this is a
> > 32-byte object, and I assume that these (too) long functions are
> > introducing enough complexity that the compiler has to have several
> > copies of them. To properly fix all this, it seems like the code could
> > use some more attention, and not just the superficial changes here.
> >
> > Anyway, I'm interested if you have any more thoughts on this. If you
> > still think this is worthwhile, I can probably just take it. (It looks
> > OK, despite the above comments.)
>
> I'd like to get this fixed in one way or another, but I'm not
> particularly attached to my specific workaround.
>
> If you don't think anyone will be able to reduce the number of
> map_word instances on the stack, please add my patch so at least
> we get rid of the warning and make it less likely to hit the bug.
OK, pushed to l2-mtd.git.
> An alternative might be to optionally move the map_word_and/clr/or/...
> operations out of line for large sizes of map_word, so we don't
> have all instances on the stack at once.
Hmm, I guess given the right amount of macro magic, it might be possible
to do this while still remaining optimal for small word sizes. But I'm
not sure anyone is really tackling these sorts of things on these
ancient drivers.
Brian
prev parent reply other threads:[~2016-04-04 6:51 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-29 12:20 [PATCH v2] mtd: avoid stack overflow in MTD CFI code Arnd Bergmann
2016-03-04 21:21 ` Brian Norris
2016-03-04 23:25 ` Arnd Bergmann
2016-03-18 17:44 ` Brian Norris
2016-03-18 20:25 ` Arnd Bergmann
2016-04-04 6:51 ` Brian Norris [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160404065134.GD13995@localhost \
--to=computersforpeace@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).