linux-kernel - Re: [PATCH 16/24] link_path

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:	Mon, 20 Apr 2015 22:51:20 +0100
From:	Al Viro <viro@...IV.linux.org.uk>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Neil Brown <neilb@...e.de>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	linux-fsdevel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH 16/24] link_path_walk: kill the recursion

On Mon, Apr 20, 2015 at 02:39:43PM -0700, Linus Torvalds wrote:
> On Mon, Apr 20, 2015 at 2:32 PM, Al Viro <viro@...iv.linux.org.uk> wrote:
> >
> > What kilobyte?  It's 9*4 pointers, IOW, 288 bytes total (assuming 64bit box).
> 
> You also said that you were going to up the recursion limit to 40.. So
> 40*3*8 bytes..

Er...  That's exactly what

||        We could reduce it further (see below), but I'm not sure it's worth
|| doing - it's not much extra complexity, but we only squeeze out ~250 bytes
|| that way, with the worst-case footprints (those are triggered by rename())
|| are around 1.4Kb (instead of about twice as much in mainline).  OTOH,
|| that extra complexity would've allowed us to get rid of the nesting limit
|| entirely (i.e. we get ELOOP when we encounter 40 symlinks, no matter in
|| which manner they are nested).  That might be worth considering...

had been about.  And yes, it is easy to implement - new nameidata flag
for "need to kfree() nd->stack",
                        if (unlikely(current->link_count >= MAX_NESTED_LINKS)) {
                                path_put_conditional(&next, nd);
                                path_put(&nd->path);
                                return -ELOOP;
                        }
                        BUG_ON(nd->depth >= MAX_NESTED_LINKS);

                        nd->depth++;
replaced with
			if (nd->depth == 2 && !(nd->flags & LOOKUP_KFREE)) {
				struct saved *p = kmalloc(41 * sizeof(*p));
				if (!p) {
					path_put_conditional(&next, nd);
					path_put(&nd->path);
					return -ENOMEM;
				}
				memcpy(p, nd->stack, 2 * sizeof(*p));
				nd->stack = p;
				nd->flags |= LOOKUP_KFREE;
			}
			nd->depth++;
with obvious logics for freeing that crap afterwards.

I really don't like the idea of putting it into nameidata, BTW - consider
e.g. rename().  We don't need the contents of that thing after the
link_path_walk() returns; no point duplicating it...
--
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/