Re: [REVIEW][PATCH 1/3] vfs: In d_path don't call d_dname on a mount point

From: NeilBrown <neilb@suse.de>
To: Al Viro <viro@ZenIV.linux.org.uk>, J.Bruce Fields <bfields@fieldses.org>
Cc: Aditya Kali <adityakali@google.com>,
	NFS <linux-nfs@vger.kernel.org>,
	Containers <containers@lists.linux-foundation.org>,
	Oleg Nesterov <oleg@redhat.com>,
	Andy Lutomirski <luto@amacapital.net>,
	Eric Dumazet <edumazet@google.com>,
	linux-fsdevel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: Re: [REVIEW][PATCH 1/3] vfs: In d_path don't call d_dname on a mount point
Date: Mon, 2 Dec 2013 16:43:59 +1100	[thread overview]
Message-ID: <20131202164359.4f4f2c94@notabene.brown> (raw)
In-Reply-To: <20131130170226.GZ10323@ZenIV.linux.org.uk>


[-- Attachment #1.1: Type: text/plain, Size: 1647 bytes --]

On Sat, 30 Nov 2013 17:02:26 +0000 Al Viro <viro@ZenIV.linux.org.uk> wrote:


> BTW, what happens if svc_export_request() ends up with pathname filling
> almost all space left, so that qword_add(bpp, blen, pth) right after the
> call of d_path() in there overwrites the beginning of d_path() output?
> Neil?  And while we are at it, handling of overflow in there looks also
> looks fishy...

In this case the returned *blen will be negative so cache_request() in
net/sunrpc/cache.h will return -EAGAIN.
cache_read() would then go into a tight loop, trying again and again to
create the request, but it will never fit in the buffer.

I guess maybe an EINVAL might help there, plus code to skip over impossible
requests.
Maybe the following.  We'd need to double check that no ->cache_request
function can fail in a way that is worth retrying, but I doubt they would.

Any thoughts Bruce?

Thanks,
NeilBrown

diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index a72de074172d..a065f827e2a3 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -748,7 +748,7 @@ static int cache_request(struct cache_detail *detail,
 
 	detail->cache_request(detail, crq->item, &bp, &len);
 	if (len < 0)
-		return -EAGAIN;
+		return -EINVAL;
 	return PAGE_SIZE - len;
 }
 
@@ -788,6 +788,11 @@ static ssize_t cache_read(struct file *filp, char __user *buf, size_t count,
 
 	if (rq->len == 0) {
 		err = cache_request(cd, rq);
+		if (err == -EINVAL) {
+			spin_lock(&queue_lock);
+			list_move(&rp->q.list, &rq->q.list);
+			spin_unlock(&queue_lock);
+		}
 		if (err < 0)
 			goto out;
 		rq->len = err;

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

[-- Attachment #2: Type: text/plain, Size: 171 bytes --]

_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers
