From patchwork Wed Feb 8 15:17:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Florian Weimer X-Patchwork-Id: 725655 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3vJPvt1tBQz9s7x for ; Thu, 9 Feb 2017 02:17:30 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.b="c194Lnlu"; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; q=dns; s=default; b= C8a35s0nLX+RQ9N3lOAX/o1XEsOgWlHYp6K4UfQFRH0QaxfTn6CLweNBriz39/MB 8+df5SUwqsq/oUsLCePCC0IFmjyHyOrEU39WXQkfqDMdCxSIHxTlTurdNMxWYzzu S0fGziCceCOaKEPbLuya1mNiYpLGYcvtLEDQAhy7YH0= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:date:to:subject:mime-version:content-type :content-transfer-encoding:message-id:from; s=default; bh=3xMZFT bu6dgDR0V5JQaUXgNkFy8=; b=c194LnlunToIWUDnGV/ljPiu/QAxdCAplKm8JS CI1Og6YZo4wYPChEGsZqCyO20EsXza4sH79/MUVVzRnqb+bS6vJC9pEXUyn9fHZK 4/GWZlPICm85qSchHUU5C1NcX5tBrnmzXAl4Ufvjfzmx4U32VaTQO5uyV7Mdr8FP q1d7g= Received: (qmail 121116 invoked by alias); 8 Feb 2017 15:17:20 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 121052 invoked by uid 89); 8 Feb 2017 15:17:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=Regular, 986, 2486, vers X-HELO: mx1.redhat.com Date: Wed, 08 Feb 2017 16:17:07 +0100 To: libc-alpha@sourceware.org Subject: [PATCH] sunrpc: Do not unregister services if not registered [BZ #5010] User-Agent: Heirloom mailx 12.5 7/5/10 MIME-Version: 1.0 Message-Id: <20170208151707.8A51C40133CDB@oldenburg.str.redhat.com> From: fweimer@redhat.com (Florian Weimer) The change in commit 718946816cf60374f9d8f674d3ed649fdb33205a has no effect because of two bugs which cancel each other out: The svc_is_mapped condition is inverted, and svc_is_mapped always returns false because the check is performed after the service has already been unregistered. As a result, pmap_unset is called unconditionally, as before. 2017-02-08 Florian Weimer [BZ #5010] * sunrpc/svc.c (svc_is_mapped): Remove. (svc_unregister): Obtain mapped status while the service is still registered. * sunrpc/Makefile [have-thread-library] (tests): Add tst-svc_register. (tst-svc_register): Link against libc.so explicitly and the thread library. * sunrpc/tst-svc_register.c: New file. diff --git a/sunrpc/Makefile b/sunrpc/Makefile index daf8a28..0249e10 100644 --- a/sunrpc/Makefile +++ b/sunrpc/Makefile @@ -98,6 +98,7 @@ xtests := tst-getmyaddr ifeq ($(have-thread-library),yes) xtests += thrsvc +tests += tst-svc_register endif ifeq ($(run-built-tests),yes) @@ -156,6 +157,8 @@ $(objpfx)tst-getmyaddr: $(common-objpfx)linkobj/libc.so $(objpfx)tst-xdrmem: $(common-objpfx)linkobj/libc.so $(objpfx)tst-xdrmem2: $(common-objpfx)linkobj/libc.so $(objpfx)tst-udp-error: $(common-objpfx)linkobj/libc.so +$(objpfx)tst-svc_register: \ + $(common-objpfx)linkobj/libc.so $(shared-thread-library) $(objpfx)rpcgen: $(addprefix $(objpfx),$(rpcgen-objs)) diff --git a/sunrpc/svc.c b/sunrpc/svc.c index 0aef2b5..03f9630 100644 --- a/sunrpc/svc.c +++ b/sunrpc/svc.c @@ -182,17 +182,6 @@ done: return s; } - -static bool_t -svc_is_mapped (rpcprog_t prog, rpcvers_t vers) -{ - struct svc_callout *prev; - register struct svc_callout *s; - s = svc_find (prog, vers, &prev); - return s!= NULL_SVC && s->sc_mapped; -} - - /* Add a service program to the callout list. The dispatch routine will be called when a rpc request for this program number comes in. */ @@ -248,6 +237,7 @@ svc_unregister (rpcprog_t prog, rpcvers_t vers) if ((s = svc_find (prog, vers, &prev)) == NULL_SVC) return; + bool is_mapped = s->sc_mapped; if (prev == NULL_SVC) svc_head = s->sc_next; @@ -257,7 +247,7 @@ svc_unregister (rpcprog_t prog, rpcvers_t vers) s->sc_next = NULL_SVC; mem_free ((char *) s, (u_int) sizeof (struct svc_callout)); /* now unregister the information with the local binder service */ - if (! svc_is_mapped (prog, vers)) + if (is_mapped) pmap_unset (prog, vers); } libc_hidden_nolink_sunrpc (svc_unregister, GLIBC_2_0) diff --git a/sunrpc/tst-svc_register.c b/sunrpc/tst-svc_register.c new file mode 100644 index 0000000..f479928 --- /dev/null +++ b/sunrpc/tst-svc_register.c @@ -0,0 +1,264 @@ +/* Test svc_register/svc_unregister rpcbind interaction. + Copyright (C) 2017 Free Software Foundation, Inc. + This file is part of the GNU C Library. + + The GNU C Library is free software; you can redistribute it and/or + modify it under the terms of the GNU Lesser General Public + License as published by the Free Software Foundation; either + version 2.1 of the License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; if not, see + . */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +/* These functions are only available as compat symbols. */ +compat_symbol_reference (libc, xdr_pmap, xdr_pmap, GLIBC_2_0); +compat_symbol_reference (libc, svc_unregister, svc_unregister, GLIBC_2_0); + +/* Server callback for the unused RPC service which is registered and + unregistered. */ +static void +server_dispatch (struct svc_req *request, SVCXPRT *transport) +{ + FAIL_EXIT1 ("server_dispatch called"); +} + +static inline const struct sockaddr_in +rpcbind_address (void) +{ + return (struct sockaddr_in) + { + .sin_family = AF_INET, + .sin_addr.s_addr = htonl (INADDR_LOOPBACK), + .sin_port = htons (PMAPPORT) + }; +} + +struct test_state +{ + bool_t set_called; + bool_t unset_called; +}; + +static bool_t +xdr_test_state (XDR *xdrs, void *data, ...) +{ + struct test_state *p = data; + return xdr_bool (xdrs, &p->set_called) + && xdr_bool (xdrs, &p->unset_called); +} + +enum +{ + /* Coordinates of our test service. */ + PROGNUM = 123, + VERSNUM = 456, + + /* Extension for this test. */ + PROC_GET_STATE_AND_EXIT = 10760 +}; + +static void +rpcbind_dispatch (struct svc_req *request, SVCXPRT *transport) +{ + static struct test_state state = { 0, }; + + if (test_verbose) + printf ("info: rpcbind request %lu\n", request->rq_proc); + + switch (request->rq_proc) + { + case PMAPPROC_SET: + case PMAPPROC_UNSET: + TEST_VERIFY (state.set_called == (request->rq_proc == PMAPPROC_UNSET)); + TEST_VERIFY (!state.unset_called); + + struct pmap query = { 0, }; + TEST_VERIFY + (svc_getargs (transport, (xdrproc_t) xdr_pmap, (void *) &query)); + if (test_verbose) + printf (" pm_prog=%lu pm_vers=%lu pm_prot=%lu pm_port=%lu\n", + query.pm_prog, query.pm_vers, query.pm_prot, query.pm_port); + TEST_VERIFY (query.pm_prog == PROGNUM); + TEST_VERIFY (query.pm_vers == VERSNUM); + + if (request->rq_proc == PMAPPROC_SET) + state.set_called = TRUE; + else + state.unset_called = TRUE; + + bool_t result = TRUE; + TEST_VERIFY (svc_sendreply (transport, + (xdrproc_t) xdr_bool, (void *) &result)); + break; + + case PROC_GET_STATE_AND_EXIT: + TEST_VERIFY (svc_sendreply (transport, + xdr_test_state, (void *) &state)); + _exit (0); + break; + + default: + FAIL_EXIT1 ("invalid rq_proc value: %lu", request->rq_proc); + } +} + +static void +run_rpcbind (int rpcbind_sock) +{ + SVCXPRT *rpcbind_transport = svcudp_create (rpcbind_sock); + TEST_VERIFY (svc_register (rpcbind_transport, PMAPPROG, PMAPVERS, + rpcbind_dispatch, + /* Do not register with rpcbind. */ + 0)); + svc_run (); +} + +static struct test_state +get_test_state (void) +{ + int socket = RPC_ANYSOCK; + struct sockaddr_in address = rpcbind_address (); + CLIENT *client = clntudp_create + (&address, PMAPPROG, PMAPVERS, (struct timeval) { 1, 0}, &socket); + struct test_state result = { 0 }; + TEST_VERIFY (clnt_call (client, PROC_GET_STATE_AND_EXIT, + (xdrproc_t) xdr_void, NULL, + xdr_test_state, (void *) &result, + ((struct timeval) { 3, 0})) + == RPC_SUCCESS); + clnt_destroy (client); + return result; +} + +struct test_server_args +{ + bool use_rpcbind; + bool use_unregister; +}; + +static void * +test_server_thread (void *closure) +{ + struct test_server_args *args = closure; + SVCXPRT *transport = svcudp_create (RPC_ANYSOCK); + int protocol; + if (args->use_rpcbind) + protocol = IPPROTO_UDP; + else + /* Do not register with rpcbind. */ + protocol = 0; + TEST_VERIFY (svc_register (transport, PROGNUM, VERSNUM, + server_dispatch, protocol)); + if (args->use_unregister) + svc_unregister (PROGNUM, VERSNUM); + SVC_DESTROY (transport); + return NULL; +} + +static int +do_test (void) +{ + support_become_root (); + support_enter_network_namespace (); + + /* Try to bind to the rpcbind port. */ + int rpcbind_sock = xsocket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); + { + struct sockaddr_in sin = rpcbind_address (); + if (bind (rpcbind_sock, (struct sockaddr *) &sin, sizeof (sin)) != 0) + { + /* If the port is not available, we cannot run this test. */ + printf ("warning: could not bind to rpcbind port %d: %m\n", + (int) PMAPPORT); + return EXIT_UNSUPPORTED; + } + } + + for (int use_thread = 0; use_thread < 2; ++use_thread) + for (int use_rpcbind = 0; use_rpcbind < 2; ++use_rpcbind) + for (int use_unregister = 0; use_unregister < 2; ++use_unregister) + { + if (test_verbose) + printf ("info: * use_thread=%d use_rpcbind=%d use_unregister=%d\n", + use_thread, use_rpcbind, use_unregister); + + pid_t svc_pid = xfork (); + if (svc_pid == 0) + { + struct test_server_args args = + { + .use_rpcbind = use_rpcbind, + .use_unregister = use_unregister, + }; + if (use_thread) + xpthread_join (xpthread_create + (NULL, test_server_thread, &args)); + else + test_server_thread (&args); + /* We cannnot use _exit here because we want to test the + process cleanup. */ + exit (0); + } + + pid_t rpcbind_pid = xfork (); + if (rpcbind_pid == 0) + run_rpcbind (rpcbind_sock); + + int status; + xwaitpid (svc_pid, &status, 0); + TEST_VERIFY (WIFEXITED (status) && WEXITSTATUS (status) == 0); + + if (!use_rpcbind) + /* Wait a bit, to see if the packet arrives on the rpcbind port. */ + usleep (300 * 1000); + + struct test_state state = get_test_state (); + if (use_rpcbind) + { + TEST_VERIFY (state.set_called); + if (use_thread || use_unregister) + TEST_VERIFY (state.unset_called); + else + /* This is arguably a bug: Regular process termination + does not unregister the service with rpcbind. */ + TEST_VERIFY (!state.unset_called); + } + else + { + TEST_VERIFY (!state.set_called); + TEST_VERIFY (!state.unset_called); + } + + xwaitpid (rpcbind_pid, &status, 0); + TEST_VERIFY (WIFEXITED (status) && WEXITSTATUS (status) == 0); + } + + return 0; +} + +#include