linux-kernel - Re: [RFC PATCH 0/2] race of lockd/nfsd inetaddr notifiers with pointers change

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]

Message-ID: <b18dff43-ebf4-f53c-1aab-9b85cb4b8354@virtuozzo.com>
Date:   Mon, 30 Oct 2017 17:55:16 +0300
From:   Vasily Averin <vvs@...tuozzo.com>
To:     linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org
Cc:     "J. Bruce Fields" <bfields@...ldses.org>,
        Jeff Layton <jlayton@...chiereds.net>,
        Scott Mayhew <smayhew@...hat.com>
Subject: Re: [RFC PATCH 0/2] race of lockd/nfsd inetaddr notifiers with
 pointers change

I've reproduced the problem both on RHEL7 and then on last mainline kernel:

1) start nfsd on host 
# service nfs start

2) create separate net and mount namespaces:
# unshare -m -n  ; mount -t nfsd nfsd /proc/fs/nfsd

3) execute screen (we need 2 consoles with newly created namespaces)
4) on first console:
# ifconfig lo up
# while : ; do ip a a  1.2.3.4/32 dev lo ; do ip a d 1.2.3.4/32 dev lo ; done

5) on second console:
# while : ; do echo 1 > /proc/fs/nfsd/threads ; sleep 1 ; echo 0 > /proc/fs/nfsd/threads ; sleep 1 ; done

Result: crash inside nfsd_inteddr_event(), see attached log.

Submitted patches have resolved the problem, patched kernel was not crashed after day of testing.

NB: during my experiments I've found "list_add double add" in set_grace_period()
and fixed it by recently submitted "[PATCH] lockd: fix lockd shutdown race with signal"

Thank you,
	Vasily Averin

On 2017-10-19 18:42, Vasily Averin wrote:
> cc: Scott Mayhew
> 
> Dear Scott,
> could you please take look at patches?
> 
> Let me describe the problem once again:
> 
> lockd_inetaddr_event()
> ...
>         if (nlmsvc_rqst) {  
> 		... 
>                 svc_age_temp_xprts_now(nlmsvc_rqst->rq_server, (struct sockaddr *)&sin);
>         }
> 
> Usually access to  nlmsvc_rqst is protected by nlmsvc_mutex 
> However lockd_inet[6]addr_event does not take the mutex,
> therefore nlmsvc_rqst can be changed during execution.
> 
> as result "if (nlmsvc_rqst)" can be passed,
> then another thread frees the memory or zeroes this pointer,
> and then svc_age_temp_xprts_now crash the host on access to already freed memory.
> 
> Moreover on initialization nlmsvc_rqst can be temporally set to ERR_PTR.
> 
> NFSD have similar issue.
> 
> On 2017-10-17 19:40, Vasily Averin wrote:
>> lockd and nfsd inet[6]addr notifiers use pointer that can be changed during execution.
>>
>> lockd_inet[6]addr_event use nlmsvc_rqst without taken nlmsvc_mutex,
>> nfsd notifier have similar trouble.
>>
>> We got few crashes from OpenVz customers on RHEL6-based kernel,
>> and I have reproduced the problem locally on this kernel.
>>
>> I was unable to reproduce the problem on new kernels,
>> however seems they are affected.
>>
>> We cannot add mutexes into notifiers because inet6addr notifiers should be atomic.
>>
>> To fix the problem I use atomic counter and waitqueue:
>> counter allows notifier to access the pointer,
>> waitqueue allows to delay stop of service until notifier is in use.
>>
>> Patches was not tested because I was unable to reproduce the problem on new kernels. 
>>
>> Please review it carefully and let me know if this can be fixed in a better way.
>>
>> Vasily Averin (2):
>>   race of lockd inetaddr notifiers with nlmsvc_rqst change
>>   race of nfsd inetaddr notifiers with nn->nfsd_serv change
>>
>>  fs/lockd/svc.c   | 16 ++++++++++++++--
>>  fs/nfsd/netns.h  |  3 +++
>>  fs/nfsd/nfsctl.c |  3 +++
>>  fs/nfsd/nfssvc.c | 14 +++++++++++---
>>  4 files changed, 31 insertions(+), 5 deletions(-)
>>


View attachment "ml2.txt" of type "text/plain" (4009 bytes)