[Lucid, Maverick, Natty] SRU: Fix panic after nfs_umount

[Stefan Bader]

On 12/09/2010 06:27 PM, Andy Whitcroft wrote:
> On Thu, Dec 09, 2010 at 04:58:41PM +0100, Stefan Bader wrote:
>> SRU justification:
>>
>> Impact: When trying to mount an export where server and client have no common
>> authentication method, the client will abort the mount by sending an advisory
>> unmount message to the server. A bug in the RPC client setup causes the sunrpc
>> code to access memory outside an allocated array, which will sooner or later
>> cause the kernel to crash.
>>
>> Fix: Patch from upstream (about to be submitted and targeted for stable too)
>> changes the setup to use the actual array size instead of a manually entered number.
>>
>> Testcase:
>>
>> Server exports a mount with an authentication method the client does not
>> support, eg.:
>> [/etc/exports] /srv/foo *(rw,sec=krb5)
>>
>> Client tries to mount this directory with no special authentication method:
>> while true; do mount <server>:/srv/foo /mnt; sync; sleep 1; done
>>
>> *Note*: This fix is not upstream yet, but is likely to go upstream in that form.
>> I just wanted to start the SRU process early due to the fact that it triggers
>> quite easily and ends in an odd and fatal mess. It is obvious enough to me and
>> has been tested locally.
>>
>> The change causing the regression has been added in the 2.6.32 time. So all
>> kernels between that and now are affected.
>>
>> -Stefan
> 
>> From dcc0a9d5d9490680ea9faa7b90de3224c3aba7e3 Mon Sep 17 00:00:00 2001
>> From: Chuck Lever <chuck.lever at oracle.com>
>> Date: Wed, 8 Dec 2010 19:07:12 -0500
>> Subject: [PATCH] NFS: Fix panic after nfs_umount()
>>
>> After a few unsuccessful NFS mount attempts in which the client and
>> server cannot agree on an authentication flavor both support, the
>> client panics.  nfs_umount() is invoked in the kernel in this case.
>>
>> Turns out this particular UMNT RPC invocation causes the RPC client to
>> write off the end of the rpc_clnt's iostat array.  This is because the
>> mount client's nrprocs field is initialized with the count of defined
>> procedures (two: MNT and UMNT), rather than the size of the client's
>> proc array (four).
>>
>> The fix is to use the same initialization technique used by most other
>> upper layer clients in the kernel.
>>
>> Introduced by commit 0b524123, which failed to update nrprocs when it
>> added support for UMNT.
>>
>> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=24302
>> BugLink: http://bugs.launchpad.net/bugs/683938
>>
>> Reported-by: Stefan Bader <stefan.bader at canonical.com>
>> Tested-by: Stefan Bader <stefan.bader at canonical.com>
>> CC: stable at kernel.org # >= 2.6.32
>> Signed-off-by: Chuck Lever <chuck.lever at oracle.com>
>> ---
>>  fs/nfs/mount_clnt.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfs/mount_clnt.c b/fs/nfs/mount_clnt.c
>> index 59047f8..50552c5 100644
>> --- a/fs/nfs/mount_clnt.c
>> +++ b/fs/nfs/mount_clnt.c
>> @@ -503,13 +503,13 @@ static struct rpc_procinfo mnt3_procedures[] = {
>>  
>>  static struct rpc_version mnt_version1 = {
>>  	.number		= 1,
>> -	.nrprocs	= 2,
>> +	.nrprocs	= ARRAY_SIZE(mnt_procedures),
>>  	.procs		= mnt_procedures,
>>  };
>>  
>>  static struct rpc_version mnt_version3 = {
>>  	.number		= 3,
>> -	.nrprocs	= 2,
>> +	.nrprocs	= ARRAY_SIZE(mnt_procedures),
>>  	.procs		= mnt3_procedures,
> 
> In this one I think the ARRAY_SIZE should reference mnt3_procedures.  A
> quick visual check says that actually they are the same size so the code
> will work as it is, but I suspect it is wrong.
> 
> Stefan as you have the upstream link could you check.
> 
> -apw

Yep, my tests were ok for that reason, but Chuck has replied and confirms that
it should be mnt3_procedures.

-Stefan