diff mbox series

[V1,3/4] net: qrtr: Change port allocation to use cyclic idr

Message ID 1598798292-5971-4-git-send-email-deesin@codeaurora.org (mailing list archive)
State New
Headers show
Series General qrtr fixes | expand

Commit Message

Deepak Kumar Singh Aug. 30, 2020, 2:38 p.m. UTC
From: Chris Lew <clew@codeaurora.org>

There is a race for clients that open sockets before the control port
is bound. If a client gets an idr that was allocated before the control
port is bound, there is a chance the previous address owner sent lookup
packets to the control port. The new address owner will get residual
responses to this the lookup packets.

Change the idr_alloc to idr_alloc_cyclic so new idr's are allocated
instead of trying to reuse the freed idrs.
---
 net/qrtr/qrtr.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

kernel test robot Aug. 30, 2020, 6:51 p.m. UTC | #1
Hi Deepak,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v5.9-rc2 next-20200828]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Deepak-Kumar-Singh/General-qrtr-fixes/20200830-224348
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 1127b219ce9481c84edad9711626d856127d5e51
config: x86_64-randconfig-a014-20200830 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project c10e63677f5d20f18010f8f68c631ddc97546f7d)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> net/qrtr/qrtr.c:747:43: warning: incompatible pointer to integer conversion passing 'u32 *' (aka 'unsigned int *') to parameter of type 'int' [-Wint-conversion]
                   rc = idr_alloc_cyclic(&qrtr_ports, ipc, &min_port,
                                                           ^~~~~~~~~
   include/linux/idr.h:117:51: note: passing argument to parameter 'start' here
   int idr_alloc_cyclic(struct idr *, void *ptr, int start, int end, gfp_t);
                                                     ^
   net/qrtr/qrtr.c:758:43: warning: incompatible pointer to integer conversion passing 'u32 *' (aka 'unsigned int *') to parameter of type 'int' [-Wint-conversion]
                   rc = idr_alloc_cyclic(&qrtr_ports, ipc, &min_port,
                                                           ^~~~~~~~~
   include/linux/idr.h:117:51: note: passing argument to parameter 'start' here
   int idr_alloc_cyclic(struct idr *, void *ptr, int start, int end, gfp_t);
                                                     ^
   2 warnings generated.

# https://github.com/0day-ci/linux/commit/66a6331807301aa59aa1355f7e21546f88cb0b2d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Deepak-Kumar-Singh/General-qrtr-fixes/20200830-224348
git checkout 66a6331807301aa59aa1355f7e21546f88cb0b2d
vim +747 net/qrtr/qrtr.c

   728	
   729	/* Assign port number to socket.
   730	 *
   731	 * Specify port in the integer pointed to by port, and it will be adjusted
   732	 * on return as necesssary.
   733	 *
   734	 * Port may be:
   735	 *   0: Assign ephemeral port in [QRTR_MIN_EPH_SOCKET, QRTR_MAX_EPH_SOCKET]
   736	 *   <QRTR_MIN_EPH_SOCKET: Specified; requires CAP_NET_ADMIN
   737	 *   >QRTR_MIN_EPH_SOCKET: Specified; available to all
   738	 */
   739	static int qrtr_port_assign(struct qrtr_sock *ipc, int *port)
   740	{
   741		u32 min_port;
   742		int rc;
   743	
   744		mutex_lock(&qrtr_port_lock);
   745		if (!*port) {
   746			min_port = QRTR_MIN_EPH_SOCKET;
 > 747			rc = idr_alloc_cyclic(&qrtr_ports, ipc, &min_port,
   748					      QRTR_MAX_EPH_SOCKET, GFP_ATOMIC);
   749			if (!rc)
   750				*port = min_port;
   751		} else if (*port < QRTR_MIN_EPH_SOCKET && !capable(CAP_NET_ADMIN)) {
   752			rc = -EACCES;
   753		} else if (*port == QRTR_PORT_CTRL) {
   754			min_port = 0;
   755			rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 0, GFP_ATOMIC);
   756		} else {
   757			min_port = *port;
   758			rc = idr_alloc_cyclic(&qrtr_ports, ipc, &min_port,
   759					      *port, GFP_ATOMIC);
   760			if (!rc)
   761				*port = min_port;
   762		}
   763		mutex_unlock(&qrtr_port_lock);
   764	
   765		if (rc == -ENOSPC)
   766			return -EADDRINUSE;
   767		else if (rc < 0)
   768			return rc;
   769	
   770		sock_hold(&ipc->sk);
   771	
   772		return 0;
   773	}
   774	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff mbox series

Patch

diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 4496b75..e2dd38e 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -744,7 +744,8 @@  static int qrtr_port_assign(struct qrtr_sock *ipc, int *port)
 	mutex_lock(&qrtr_port_lock);
 	if (!*port) {
 		min_port = QRTR_MIN_EPH_SOCKET;
-		rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, QRTR_MAX_EPH_SOCKET, GFP_ATOMIC);
+		rc = idr_alloc_cyclic(&qrtr_ports, ipc, &min_port,
+				      QRTR_MAX_EPH_SOCKET, GFP_ATOMIC);
 		if (!rc)
 			*port = min_port;
 	} else if (*port < QRTR_MIN_EPH_SOCKET && !capable(CAP_NET_ADMIN)) {
@@ -754,7 +755,8 @@  static int qrtr_port_assign(struct qrtr_sock *ipc, int *port)
 		rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, 0, GFP_ATOMIC);
 	} else {
 		min_port = *port;
-		rc = idr_alloc_u32(&qrtr_ports, ipc, &min_port, *port, GFP_ATOMIC);
+		rc = idr_alloc_cyclic(&qrtr_ports, ipc, &min_port,
+				      *port, GFP_ATOMIC);
 		if (!rc)
 			*port = min_port;
 	}