From: Al Viro <viro@ZenIV.linux.org.uk>
To: "J. Bruce Fields" <bfields@fieldses.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
linux-nfs@vger.kernel.org, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: file locking fix for 3.2
Date: Sat, 24 Dec 2011 22:55:25 +0000 [thread overview]
Message-ID: <20111224225525.GR23916@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20111224215012.GA23495@fieldses.org>
On Sat, Dec 24, 2011 at 04:50:12PM -0500, J. Bruce Fields wrote:
> locks: fix null dereference on lease-break failure path
>
> Commit 778fc546f749c588aa2f6cd50215d2715c374252 "locks: fix tracking of
> inprogress lease breaks" introduced a null dereference on failure to
> allocate memory.
>
> This means an open (without O_NONBLOCK set) on a file with a lease
> applied (generally only done when Samba or nfsd (with v4) is running)
> could crash if a kmalloc() fails.
NULL? AFAICS, lease_alloc() returns ERR_PTR() on failure... I really
don't like the look of that code, TBH - at the very least it needs to
be commented a lot. E.g. the rules for calling or not calling ->lm_break()
are really not obvious - AFAICS, we do that if
i_have_this_lease || (mode & O_NONBLOCK)
is true *or* if allocation has succeeded. The former condition is what'll
end up with -EWOULDBLOCK; I can understand not wanting to return that in
preference to -ENOMEM, but... Do we want to skip ->lm_break() stuff only
in case of allocation failures that won't be overridden by -EWOULDBLOCK?
next prev parent reply other threads:[~2011-12-24 22:55 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-12-24 21:50 file locking fix for 3.2 J. Bruce Fields
2011-12-24 22:55 ` Al Viro [this message]
2011-12-24 23:50 ` J. Bruce Fields
2011-12-24 23:50 ` J. Bruce Fields
2011-12-25 0:05 ` Al Viro
2011-12-25 18:19 ` J. Bruce Fields
2011-12-26 18:37 ` Linus Torvalds
2011-12-26 20:18 ` J. Bruce Fields
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=20111224225525.GR23916@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=bfields@fieldses.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-nfs@vger.kernel.org \
--cc=torvalds@linux-foundation.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.