All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC, DROP}TAG ioctls for tag control
@ 2022-02-13  1:46 kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-02-13  1:46 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 24793 bytes --]

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20220208094617.3675511-6-jk@codeconstruct.com.au>
References: <20220208094617.3675511-6-jk@codeconstruct.com.au>
TO: Jeremy Kerr <jk@codeconstruct.com.au>

Hi Jeremy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jeremy-Kerr/MCTP-tag-control-interface/20220208-195325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git c3e676b98326a419f30dd5d956c68fc33323f4fd
:::::: branch date: 5 days ago
:::::: commit date: 5 days ago
config: x86_64-randconfig-c007 (https://download.01.org/0day-ci/archive/20220213/202202130926.bzFbDbv2-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project e8bff9ae54a55b4dbfeb6ba55f723abbd81bf494)
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
        # https://github.com/0day-ci/linux/commit/a1d553f399d7457bd3e455cd3f5e10dddb4bc2bf
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jeremy-Kerr/MCTP-tag-control-interface/20220208-195325
        git checkout a1d553f399d7457bd3e455cd3f5e10dddb4bc2bf
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer 

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


clang-analyzer warnings: (new ones prefixed by >>)
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:335:2: note: Taking false branch
           if (skb->len < sizeof(struct mctp_hdr) + 1)
           ^
   net/mctp/route.c:342:6: note: Assuming field 'ver' is equal to 1
           if (mh->ver != 1)
               ^~~~~~~~~~~~
   net/mctp/route.c:342:2: note: Taking false branch
           if (mh->ver != 1)
           ^
   net/mctp/route.c:355:6: note: Assuming the condition is true
           if (flags & MCTP_HDR_FLAG_SOM) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:355:2: note: Taking true branch
           if (flags & MCTP_HDR_FLAG_SOM) {
           ^
   net/mctp/route.c:356:7: note: 'key' is null
                   if (key) {
                       ^~~
   net/mctp/route.c:356:3: note: Taking false branch
                   if (key) {
                   ^
   net/mctp/route.c:365:8: note: 'key' is null
                           if (key) {
                               ^~~
   net/mctp/route.c:365:4: note: Taking false branch
                           if (key) {
                           ^
   net/mctp/route.c:374:8: note: 'key' is null
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                        ^~~
   net/mctp/route.c:374:7: note: Left side of '&&' is true
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                       ^
   net/mctp/route.c:374:16: note: 'msk' is null
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                                ^~~
   net/mctp/route.c:374:7: note: Left side of '&&' is true
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                       ^
   net/mctp/route.c:374:24: note: Assuming the condition is true
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                                        ^~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:374:3: note: Taking true branch
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                   ^
   net/mctp/route.c:377:8: note: 'msk' is non-null
                   if (!msk) {
                        ^~~
   net/mctp/route.c:377:3: note: Taking false branch
                   if (!msk) {
                   ^
   net/mctp/route.c:385:7: note: Assuming the condition is false
                   if (flags & MCTP_HDR_FLAG_EOM) {
                       ^~~~~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:385:3: note: Taking false branch
                   if (flags & MCTP_HDR_FLAG_EOM) {
                   ^
   net/mctp/route.c:402:8: note: 'key' is null
                   if (!key) {
                        ^~~
   net/mctp/route.c:402:3: note: Taking true branch
                   if (!key) {
                   ^
   net/mctp/route.c:403:10: note: Calling 'mctp_key_alloc'
                           key = mctp_key_alloc(msk, mh->dest, mh->src,
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:140:6: note: Assuming 'key' is non-null
           if (!key)
               ^~~~
   net/mctp/route.c:140:2: note: Taking false branch
           if (!key)
           ^
   net/mctp/route.c:148:2: note: Loop condition is false.  Exiting loop
           spin_lock_init(&key->lock);
           ^
   include/linux/spinlock.h:329:35: note: expanded from macro 'spin_lock_init'
   # define spin_lock_init(lock)                                   \
                                                                   ^
   net/mctp/route.c:403:10: note: Returning from 'mctp_key_alloc'
                           key = mctp_key_alloc(msk, mh->dest, mh->src,
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:405:9: note: 'key' is non-null
                           if (!key) {
                                ^~~
   net/mctp/route.c:405:4: note: Taking false branch
                           if (!key) {
                           ^
   net/mctp/route.c:422:8: note: Assuming 'rc' is not equal to 0
                           if (rc)
                               ^~
   net/mctp/route.c:422:4: note: Taking true branch
                           if (rc)
                           ^
   net/mctp/route.c:423:5: note: Memory is released
                                   kfree(key);
                                   ^~~~~~~~~~
   net/mctp/route.c:425:4: note: Use of memory after it is freed
                           trace_mctp_key_acquire(key);
                           ^                      ~~~
>> net/mctp/route.c:458:4: warning: Value stored to 'msk' is never read [clang-analyzer-deadcode.DeadStores]
                           msk = container_of(key->sk, struct mctp_sock, sk);
                           ^
   net/mctp/route.c:458:4: note: Value stored to 'msk' is never read
   Suppressed 11 warnings (10 in non-user code, 1 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   3 warnings generated.
   Suppressed 3 warnings (3 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   5 warnings generated.
   Suppressed 5 warnings (5 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   6 warnings generated.
   drivers/mmc/host/wbsd.c:1289:4: warning: Value stored to 'id' is never read [clang-analyzer-deadcode.DeadStores]
                           id = 0xFFFF;
                           ^    ~~~~~~
   drivers/mmc/host/wbsd.c:1289:4: note: Value stored to 'id' is never read
                           id = 0xFFFF;
                           ^    ~~~~~~
   Suppressed 5 warnings (5 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   7 warnings generated.
   drivers/mmc/host/mtk-sd.c:680:2: warning: Value stored to 'sg' is never read [clang-analyzer-deadcode.DeadStores]
           sg = data->sg;
           ^    ~~~~~~~~
   drivers/mmc/host/mtk-sd.c:680:2: note: Value stored to 'sg' is never read
           sg = data->sg;
           ^    ~~~~~~~~
   drivers/mmc/host/mtk-sd.c:1054:2: warning: Value stored to 'read' is never read [clang-analyzer-deadcode.DeadStores]
           read = data->flags & MMC_DATA_READ;
           ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/mmc/host/mtk-sd.c:1054:2: note: Value stored to 'read' is never read
           read = data->flags & MMC_DATA_READ;
           ^      ~~~~~~~~~~~~~~~~~~~~~~~~~~~
   Suppressed 5 warnings (5 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   11 warnings generated.
   drivers/mmc/host/usdhi6rol0.c:397:26: warning: Access to field 'data' results in a dereference of a null pointer (loaded from field 'mrq') [clang-analyzer-core.NullDereference]
           struct mmc_data *data = host->mrq->data;
                                   ^
   drivers/mmc/host/usdhi6rol0.c:1686:26: note: Assuming 'mrq' is null
           struct mmc_data *data = mrq ? mrq->data : NULL;
                                   ^~~
   drivers/mmc/host/usdhi6rol0.c:1686:26: note: '?' condition is false
   drivers/mmc/host/usdhi6rol0.c:1689:2: note: Loop condition is false.  Exiting loop
           dev_warn(mmc_dev(host->mmc),
           ^
   include/linux/dev_printk.h:146:2: note: expanded from macro 'dev_warn'
           dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
           ^
   include/linux/dev_printk.h:109:3: note: expanded from macro 'dev_printk_index_wrap'
                   dev_printk_index_emit(level, fmt);                      \
                   ^
   include/linux/dev_printk.h:105:2: note: expanded from macro 'dev_printk_index_emit'
           printk_index_subsys_emit("%s %s: ", level, fmt)
           ^
   include/linux/printk.h:413:2: note: expanded from macro 'printk_index_subsys_emit'
           __printk_index_emit(fmt, level, subsys_fmt_prefix)
           ^
   include/linux/printk.h:392:34: note: expanded from macro '__printk_index_emit'
   #define __printk_index_emit(...) do {} while (0)
                                    ^
   drivers/mmc/host/usdhi6rol0.c:1691:4: note: Assuming field 'dma_active' is false
                    host->dma_active ? "DMA" : "PIO",
                    ^
   include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
           dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
                                                                               ^~~~~~~~~~~
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
                   _p_func(dev, fmt, ##__VA_ARGS__);                       \
                                       ^~~~~~~~~~~
   drivers/mmc/host/usdhi6rol0.c:1691:4: note: '?' condition is false
                    host->dma_active ? "DMA" : "PIO",
                    ^
   drivers/mmc/host/usdhi6rol0.c:1692:16: note: 'mrq' is null
                    host->wait, mrq ? mrq->cmd->opcode : -1,
                                ^
   include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
           dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
                                                                               ^~~~~~~~~~~
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
                   _p_func(dev, fmt, ##__VA_ARGS__);                       \
                                       ^~~~~~~~~~~
   drivers/mmc/host/usdhi6rol0.c:1692:16: note: '?' condition is false
                    host->wait, mrq ? mrq->cmd->opcode : -1,
                                ^
   drivers/mmc/host/usdhi6rol0.c:1693:4: note: Calling 'usdhi6_read'
                    usdhi6_read(host, USDHI6_SD_INFO1),
                    ^
   include/linux/dev_printk.h:146:70: note: expanded from macro 'dev_warn'
           dev_printk_index_wrap(_dev_warn, KERN_WARNING, dev, dev_fmt(fmt), ##__VA_ARGS__)
                                                                               ^~~~~~~~~~~
   include/linux/dev_printk.h:110:23: note: expanded from macro 'dev_printk_index_wrap'
                   _p_func(dev, fmt, ##__VA_ARGS__);                       \
                                       ^~~~~~~~~~~
   drivers/mmc/host/usdhi6rol0.c:226:2: note: Taking false branch
           dev_vdbg(mmc_dev(host->mmc), "%s(0x%p + 0x%x) = 0x%x\n", __func__,
           ^

vim +/msk +458 net/mctp/route.c

4a992bbd365094 Jeremy Kerr   2021-07-29  315  
889b7da23abf92 Jeremy Kerr   2021-07-29  316  static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
889b7da23abf92 Jeremy Kerr   2021-07-29  317  {
833ef3b91de692 Jeremy Kerr   2021-07-29  318  	struct net *net = dev_net(skb->dev);
833ef3b91de692 Jeremy Kerr   2021-07-29  319  	struct mctp_sk_key *key;
833ef3b91de692 Jeremy Kerr   2021-07-29  320  	struct mctp_sock *msk;
833ef3b91de692 Jeremy Kerr   2021-07-29  321  	struct mctp_hdr *mh;
4a992bbd365094 Jeremy Kerr   2021-07-29  322  	unsigned long f;
4a992bbd365094 Jeremy Kerr   2021-07-29  323  	u8 tag, flags;
4a992bbd365094 Jeremy Kerr   2021-07-29  324  	int rc;
833ef3b91de692 Jeremy Kerr   2021-07-29  325  
833ef3b91de692 Jeremy Kerr   2021-07-29  326  	msk = NULL;
4a992bbd365094 Jeremy Kerr   2021-07-29  327  	rc = -EINVAL;
833ef3b91de692 Jeremy Kerr   2021-07-29  328  
833ef3b91de692 Jeremy Kerr   2021-07-29  329  	/* we may be receiving a locally-routed packet; drop source sk
833ef3b91de692 Jeremy Kerr   2021-07-29  330  	 * accounting
833ef3b91de692 Jeremy Kerr   2021-07-29  331  	 */
833ef3b91de692 Jeremy Kerr   2021-07-29  332  	skb_orphan(skb);
833ef3b91de692 Jeremy Kerr   2021-07-29  333  
833ef3b91de692 Jeremy Kerr   2021-07-29  334  	/* ensure we have enough data for a header and a type */
833ef3b91de692 Jeremy Kerr   2021-07-29  335  	if (skb->len < sizeof(struct mctp_hdr) + 1)
4a992bbd365094 Jeremy Kerr   2021-07-29  336  		goto out;
833ef3b91de692 Jeremy Kerr   2021-07-29  337  
833ef3b91de692 Jeremy Kerr   2021-07-29  338  	/* grab header, advance data ptr */
833ef3b91de692 Jeremy Kerr   2021-07-29  339  	mh = mctp_hdr(skb);
833ef3b91de692 Jeremy Kerr   2021-07-29  340  	skb_pull(skb, sizeof(struct mctp_hdr));
833ef3b91de692 Jeremy Kerr   2021-07-29  341  
833ef3b91de692 Jeremy Kerr   2021-07-29  342  	if (mh->ver != 1)
4a992bbd365094 Jeremy Kerr   2021-07-29  343  		goto out;
833ef3b91de692 Jeremy Kerr   2021-07-29  344  
4a992bbd365094 Jeremy Kerr   2021-07-29  345  	flags = mh->flags_seq_tag & (MCTP_HDR_FLAG_SOM | MCTP_HDR_FLAG_EOM);
4a992bbd365094 Jeremy Kerr   2021-07-29  346  	tag = mh->flags_seq_tag & (MCTP_HDR_TAG_MASK | MCTP_HDR_FLAG_TO);
833ef3b91de692 Jeremy Kerr   2021-07-29  347  
833ef3b91de692 Jeremy Kerr   2021-07-29  348  	rcu_read_lock();
4a992bbd365094 Jeremy Kerr   2021-07-29  349  
73c618456dc5cf Jeremy Kerr   2021-09-29  350  	/* lookup socket / reasm context, exactly matching (src,dest,tag).
73c618456dc5cf Jeremy Kerr   2021-09-29  351  	 * we hold a ref on the key, and key->lock held.
73c618456dc5cf Jeremy Kerr   2021-09-29  352  	 */
73c618456dc5cf Jeremy Kerr   2021-09-29  353  	key = mctp_lookup_key(net, skb, mh->src, &f);
833ef3b91de692 Jeremy Kerr   2021-07-29  354  
4a992bbd365094 Jeremy Kerr   2021-07-29  355  	if (flags & MCTP_HDR_FLAG_SOM) {
4a992bbd365094 Jeremy Kerr   2021-07-29  356  		if (key) {
4a992bbd365094 Jeremy Kerr   2021-07-29  357  			msk = container_of(key->sk, struct mctp_sock, sk);
4a992bbd365094 Jeremy Kerr   2021-07-29  358  		} else {
4a992bbd365094 Jeremy Kerr   2021-07-29  359  			/* first response to a broadcast? do a more general
4a992bbd365094 Jeremy Kerr   2021-07-29  360  			 * key lookup to find the socket, but don't use this
4a992bbd365094 Jeremy Kerr   2021-07-29  361  			 * key for reassembly - we'll create a more specific
4a992bbd365094 Jeremy Kerr   2021-07-29  362  			 * one for future packets if required (ie, !EOM).
4a992bbd365094 Jeremy Kerr   2021-07-29  363  			 */
73c618456dc5cf Jeremy Kerr   2021-09-29  364  			key = mctp_lookup_key(net, skb, MCTP_ADDR_ANY, &f);
4a992bbd365094 Jeremy Kerr   2021-07-29  365  			if (key) {
4a992bbd365094 Jeremy Kerr   2021-07-29  366  				msk = container_of(key->sk,
4a992bbd365094 Jeremy Kerr   2021-07-29  367  						   struct mctp_sock, sk);
73c618456dc5cf Jeremy Kerr   2021-09-29  368  				spin_unlock_irqrestore(&key->lock, f);
73c618456dc5cf Jeremy Kerr   2021-09-29  369  				mctp_key_unref(key);
4a992bbd365094 Jeremy Kerr   2021-07-29  370  				key = NULL;
4a992bbd365094 Jeremy Kerr   2021-07-29  371  			}
4a992bbd365094 Jeremy Kerr   2021-07-29  372  		}
833ef3b91de692 Jeremy Kerr   2021-07-29  373  
4a992bbd365094 Jeremy Kerr   2021-07-29  374  		if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
833ef3b91de692 Jeremy Kerr   2021-07-29  375  			msk = mctp_lookup_bind(net, skb);
833ef3b91de692 Jeremy Kerr   2021-07-29  376  
4a992bbd365094 Jeremy Kerr   2021-07-29  377  		if (!msk) {
4a992bbd365094 Jeremy Kerr   2021-07-29  378  			rc = -ENOENT;
4a992bbd365094 Jeremy Kerr   2021-07-29  379  			goto out_unlock;
4a992bbd365094 Jeremy Kerr   2021-07-29  380  		}
833ef3b91de692 Jeremy Kerr   2021-07-29  381  
4a992bbd365094 Jeremy Kerr   2021-07-29  382  		/* single-packet message? deliver to socket, clean up any
4a992bbd365094 Jeremy Kerr   2021-07-29  383  		 * pending key.
4a992bbd365094 Jeremy Kerr   2021-07-29  384  		 */
4a992bbd365094 Jeremy Kerr   2021-07-29  385  		if (flags & MCTP_HDR_FLAG_EOM) {
833ef3b91de692 Jeremy Kerr   2021-07-29  386  			sock_queue_rcv_skb(&msk->sk, skb);
4a992bbd365094 Jeremy Kerr   2021-07-29  387  			if (key) {
4a992bbd365094 Jeremy Kerr   2021-07-29  388  				/* we've hit a pending reassembly; not much we
4a992bbd365094 Jeremy Kerr   2021-07-29  389  				 * can do but drop it
4a992bbd365094 Jeremy Kerr   2021-07-29  390  				 */
a1d553f399d745 Matt Johnston 2022-02-08  391  				__mctp_key_done_in(key, net, f,
4f9e1ba6de45aa Jeremy Kerr   2021-09-29  392  						   MCTP_TRACE_KEY_REPLIED);
73c618456dc5cf Jeremy Kerr   2021-09-29  393  				key = NULL;
4a992bbd365094 Jeremy Kerr   2021-07-29  394  			}
4a992bbd365094 Jeremy Kerr   2021-07-29  395  			rc = 0;
4a992bbd365094 Jeremy Kerr   2021-07-29  396  			goto out_unlock;
4a992bbd365094 Jeremy Kerr   2021-07-29  397  		}
833ef3b91de692 Jeremy Kerr   2021-07-29  398  
4a992bbd365094 Jeremy Kerr   2021-07-29  399  		/* broadcast response or a bind() - create a key for further
4a992bbd365094 Jeremy Kerr   2021-07-29  400  		 * packets for this message
4a992bbd365094 Jeremy Kerr   2021-07-29  401  		 */
4a992bbd365094 Jeremy Kerr   2021-07-29  402  		if (!key) {
4a992bbd365094 Jeremy Kerr   2021-07-29  403  			key = mctp_key_alloc(msk, mh->dest, mh->src,
4a992bbd365094 Jeremy Kerr   2021-07-29  404  					     tag, GFP_ATOMIC);
4a992bbd365094 Jeremy Kerr   2021-07-29  405  			if (!key) {
4a992bbd365094 Jeremy Kerr   2021-07-29  406  				rc = -ENOMEM;
4a992bbd365094 Jeremy Kerr   2021-07-29  407  				goto out_unlock;
4a992bbd365094 Jeremy Kerr   2021-07-29  408  			}
833ef3b91de692 Jeremy Kerr   2021-07-29  409  
73c618456dc5cf Jeremy Kerr   2021-09-29  410  			/* we can queue without the key lock here, as the
4a992bbd365094 Jeremy Kerr   2021-07-29  411  			 * key isn't observable yet
4a992bbd365094 Jeremy Kerr   2021-07-29  412  			 */
4a992bbd365094 Jeremy Kerr   2021-07-29  413  			mctp_frag_queue(key, skb);
4a992bbd365094 Jeremy Kerr   2021-07-29  414  
4a992bbd365094 Jeremy Kerr   2021-07-29  415  			/* if the key_add fails, we've raced with another
4a992bbd365094 Jeremy Kerr   2021-07-29  416  			 * SOM packet with the same src, dest and tag. There's
4a992bbd365094 Jeremy Kerr   2021-07-29  417  			 * no way to distinguish future packets, so all we
4a992bbd365094 Jeremy Kerr   2021-07-29  418  			 * can do is drop; we'll free the skb on exit from
4a992bbd365094 Jeremy Kerr   2021-07-29  419  			 * this function.
4a992bbd365094 Jeremy Kerr   2021-07-29  420  			 */
4a992bbd365094 Jeremy Kerr   2021-07-29  421  			rc = mctp_key_add(key, msk);
4a992bbd365094 Jeremy Kerr   2021-07-29  422  			if (rc)
4a992bbd365094 Jeremy Kerr   2021-07-29  423  				kfree(key);
4a992bbd365094 Jeremy Kerr   2021-07-29  424  
4f9e1ba6de45aa Jeremy Kerr   2021-09-29  425  			trace_mctp_key_acquire(key);
4f9e1ba6de45aa Jeremy Kerr   2021-09-29  426  
73c618456dc5cf Jeremy Kerr   2021-09-29  427  			/* we don't need to release key->lock on exit */
0b93aed2842d95 Matt Johnston 2021-10-14  428  			mctp_key_unref(key);
73c618456dc5cf Jeremy Kerr   2021-09-29  429  			key = NULL;
4a992bbd365094 Jeremy Kerr   2021-07-29  430  
73c618456dc5cf Jeremy Kerr   2021-09-29  431  		} else {
4a992bbd365094 Jeremy Kerr   2021-07-29  432  			if (key->reasm_head || key->reasm_dead) {
4a992bbd365094 Jeremy Kerr   2021-07-29  433  				/* duplicate start? drop everything */
a1d553f399d745 Matt Johnston 2022-02-08  434  				__mctp_key_done_in(key, net, f,
4f9e1ba6de45aa Jeremy Kerr   2021-09-29  435  						   MCTP_TRACE_KEY_INVALIDATED);
4a992bbd365094 Jeremy Kerr   2021-07-29  436  				rc = -EEXIST;
73c618456dc5cf Jeremy Kerr   2021-09-29  437  				key = NULL;
4a992bbd365094 Jeremy Kerr   2021-07-29  438  			} else {
4a992bbd365094 Jeremy Kerr   2021-07-29  439  				rc = mctp_frag_queue(key, skb);
4a992bbd365094 Jeremy Kerr   2021-07-29  440  			}
4a992bbd365094 Jeremy Kerr   2021-07-29  441  		}
4a992bbd365094 Jeremy Kerr   2021-07-29  442  
4a992bbd365094 Jeremy Kerr   2021-07-29  443  	} else if (key) {
4a992bbd365094 Jeremy Kerr   2021-07-29  444  		/* this packet continues a previous message; reassemble
4a992bbd365094 Jeremy Kerr   2021-07-29  445  		 * using the message-specific key
4a992bbd365094 Jeremy Kerr   2021-07-29  446  		 */
4a992bbd365094 Jeremy Kerr   2021-07-29  447  
4a992bbd365094 Jeremy Kerr   2021-07-29  448  		/* we need to be continuing an existing reassembly... */
4a992bbd365094 Jeremy Kerr   2021-07-29  449  		if (!key->reasm_head)
4a992bbd365094 Jeremy Kerr   2021-07-29  450  			rc = -EINVAL;
4a992bbd365094 Jeremy Kerr   2021-07-29  451  		else
4a992bbd365094 Jeremy Kerr   2021-07-29  452  			rc = mctp_frag_queue(key, skb);
4a992bbd365094 Jeremy Kerr   2021-07-29  453  
4a992bbd365094 Jeremy Kerr   2021-07-29  454  		/* end of message? deliver to socket, and we're done with
4a992bbd365094 Jeremy Kerr   2021-07-29  455  		 * the reassembly/response key
4a992bbd365094 Jeremy Kerr   2021-07-29  456  		 */
4a992bbd365094 Jeremy Kerr   2021-07-29  457  		if (!rc && flags & MCTP_HDR_FLAG_EOM) {
a1d553f399d745 Matt Johnston 2022-02-08 @458  			msk = container_of(key->sk, struct mctp_sock, sk);
4a992bbd365094 Jeremy Kerr   2021-07-29  459  			sock_queue_rcv_skb(key->sk, key->reasm_head);
4a992bbd365094 Jeremy Kerr   2021-07-29  460  			key->reasm_head = NULL;
a1d553f399d745 Matt Johnston 2022-02-08  461  			__mctp_key_done_in(key, net, f, MCTP_TRACE_KEY_REPLIED);
73c618456dc5cf Jeremy Kerr   2021-09-29  462  			key = NULL;
4a992bbd365094 Jeremy Kerr   2021-07-29  463  		}
4a992bbd365094 Jeremy Kerr   2021-07-29  464  
4a992bbd365094 Jeremy Kerr   2021-07-29  465  	} else {
4a992bbd365094 Jeremy Kerr   2021-07-29  466  		/* not a start, no matching key */
4a992bbd365094 Jeremy Kerr   2021-07-29  467  		rc = -ENOENT;
4a992bbd365094 Jeremy Kerr   2021-07-29  468  	}
4a992bbd365094 Jeremy Kerr   2021-07-29  469  
4a992bbd365094 Jeremy Kerr   2021-07-29  470  out_unlock:
833ef3b91de692 Jeremy Kerr   2021-07-29  471  	rcu_read_unlock();
73c618456dc5cf Jeremy Kerr   2021-09-29  472  	if (key) {
73c618456dc5cf Jeremy Kerr   2021-09-29  473  		spin_unlock_irqrestore(&key->lock, f);
73c618456dc5cf Jeremy Kerr   2021-09-29  474  		mctp_key_unref(key);
73c618456dc5cf Jeremy Kerr   2021-09-29  475  	}
4a992bbd365094 Jeremy Kerr   2021-07-29  476  out:
4a992bbd365094 Jeremy Kerr   2021-07-29  477  	if (rc)
889b7da23abf92 Jeremy Kerr   2021-07-29  478  		kfree_skb(skb);
4a992bbd365094 Jeremy Kerr   2021-07-29  479  	return rc;
4a992bbd365094 Jeremy Kerr   2021-07-29  480  }
4a992bbd365094 Jeremy Kerr   2021-07-29  481  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC, DROP}TAG ioctls for tag control
@ 2022-02-09 17:43 kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-02-09 17:43 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 23362 bytes --]

CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20220208094617.3675511-6-jk@codeconstruct.com.au>
References: <20220208094617.3675511-6-jk@codeconstruct.com.au>
TO: Jeremy Kerr <jk@codeconstruct.com.au>

Hi Jeremy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jeremy-Kerr/MCTP-tag-control-interface/20220208-195325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git c3e676b98326a419f30dd5d956c68fc33323f4fd
:::::: branch date: 30 hours ago
:::::: commit date: 30 hours ago
config: x86_64-randconfig-c007 (https://download.01.org/0day-ci/archive/20220210/202202100128.Jug3BevE-lkp(a)intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project e8bff9ae54a55b4dbfeb6ba55f723abbd81bf494)
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
        # https://github.com/0day-ci/linux/commit/a1d553f399d7457bd3e455cd3f5e10dddb4bc2bf
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jeremy-Kerr/MCTP-tag-control-interface/20220208-195325
        git checkout a1d553f399d7457bd3e455cd3f5e10dddb4bc2bf
        # save the config file to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 clang-analyzer 

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


clang-analyzer warnings: (new ones prefixed by >>)
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:335:2: note: Taking false branch
           if (skb->len < sizeof(struct mctp_hdr) + 1)
           ^
   net/mctp/route.c:342:6: note: Assuming field 'ver' is equal to 1
           if (mh->ver != 1)
               ^~~~~~~~~~~~
   net/mctp/route.c:342:2: note: Taking false branch
           if (mh->ver != 1)
           ^
   net/mctp/route.c:355:6: note: Assuming the condition is true
           if (flags & MCTP_HDR_FLAG_SOM) {
               ^~~~~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:355:2: note: Taking true branch
           if (flags & MCTP_HDR_FLAG_SOM) {
           ^
   net/mctp/route.c:356:7: note: 'key' is null
                   if (key) {
                       ^~~
   net/mctp/route.c:356:3: note: Taking false branch
                   if (key) {
                   ^
   net/mctp/route.c:365:8: note: 'key' is null
                           if (key) {
                               ^~~
   net/mctp/route.c:365:4: note: Taking false branch
                           if (key) {
                           ^
   net/mctp/route.c:374:8: note: 'key' is null
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                        ^~~
   net/mctp/route.c:374:7: note: Left side of '&&' is true
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                       ^
   net/mctp/route.c:374:16: note: 'msk' is null
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                                ^~~
   net/mctp/route.c:374:7: note: Left side of '&&' is true
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                       ^
   net/mctp/route.c:374:24: note: Assuming the condition is true
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                                        ^~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:374:3: note: Taking true branch
                   if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
                   ^
   net/mctp/route.c:377:8: note: 'msk' is non-null
                   if (!msk) {
                        ^~~
   net/mctp/route.c:377:3: note: Taking false branch
                   if (!msk) {
                   ^
   net/mctp/route.c:385:7: note: Assuming the condition is false
                   if (flags & MCTP_HDR_FLAG_EOM) {
                       ^~~~~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:385:3: note: Taking false branch
                   if (flags & MCTP_HDR_FLAG_EOM) {
                   ^
   net/mctp/route.c:402:8: note: 'key' is null
                   if (!key) {
                        ^~~
   net/mctp/route.c:402:3: note: Taking true branch
                   if (!key) {
                   ^
   net/mctp/route.c:403:10: note: Calling 'mctp_key_alloc'
                           key = mctp_key_alloc(msk, mh->dest, mh->src,
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:140:6: note: Assuming 'key' is non-null
           if (!key)
               ^~~~
   net/mctp/route.c:140:2: note: Taking false branch
           if (!key)
           ^
   net/mctp/route.c:148:2: note: Loop condition is false.  Exiting loop
           spin_lock_init(&key->lock);
           ^
   include/linux/spinlock.h:329:35: note: expanded from macro 'spin_lock_init'
   # define spin_lock_init(lock)                                   \
                                                                   ^
   net/mctp/route.c:403:10: note: Returning from 'mctp_key_alloc'
                           key = mctp_key_alloc(msk, mh->dest, mh->src,
                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   net/mctp/route.c:405:9: note: 'key' is non-null
                           if (!key) {
                                ^~~
   net/mctp/route.c:405:4: note: Taking false branch
                           if (!key) {
                           ^
   net/mctp/route.c:422:8: note: Assuming 'rc' is not equal to 0
                           if (rc)
                               ^~
   net/mctp/route.c:422:4: note: Taking true branch
                           if (rc)
                           ^
   net/mctp/route.c:423:5: note: Memory is released
                                   kfree(key);
                                   ^~~~~~~~~~
   net/mctp/route.c:425:4: note: Use of memory after it is freed
                           trace_mctp_key_acquire(key);
                           ^                      ~~~
>> net/mctp/route.c:458:4: warning: Value stored to 'msk' is never read [clang-analyzer-deadcode.DeadStores]
                           msk = container_of(key->sk, struct mctp_sock, sk);
                           ^
   net/mctp/route.c:458:4: note: Value stored to 'msk' is never read
   Suppressed 11 warnings (10 in non-user code, 1 with check filters).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   10 warnings generated.
   Suppressed 10 warnings (10 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   10 warnings generated.
   Suppressed 10 warnings (10 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   10 warnings generated.
   Suppressed 10 warnings (10 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   10 warnings generated.
   Suppressed 10 warnings (10 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   5 warnings generated.
   Suppressed 5 warnings (5 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   4 warnings generated.
   Suppressed 4 warnings (4 in non-user code).
   Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
   14 warnings generated.
   drivers/char/ipmi/ipmi_ssif.c:198:8: warning: Excessive padding in 'struct ssif_info' (35 padding bytes, where 3 is optimal). 
   Optimal fields order: 
   intf, 
   waiting_msg, 
   curr_msg, 
   ssif_debug, 
   addr_info, 
   client, 
   done_handler, 
   thread, 
   i2c_data, 
   watch_timeout, 
   multi_data, 
   lock, 
   retry_timer, 
   watch_timer, 
   handlers, 
   wake_thread, 
   ssif_state, 
   addr_source, 
   rtc_us_timer, 
   data_len, 
   i2c_read_write, 
   i2c_command, 
   i2c_size, 
   retries_left, 
   multi_support, 
   supports_pec, 
   multi_len, 
   multi_pos, 
   stats, 
   msg_flags, 
   global_enables, 
   has_event_buffer, 
   supports_alert, 
   got_alert, 
   waiting_alert, 
   req_events, 
   req_flags, 
   stopping, 
   max_xmit_msg_size, 
   max_recv_msg_size, 
   cmd8_works, 
   recv, 
   data, 
   consider reordering the fields or adding explicit padding members [clang-analyzer-optin.performance.Padding]

vim +/msk +458 net/mctp/route.c

4a992bbd3650947 Jeremy Kerr   2021-07-29  315  
889b7da23abf92f Jeremy Kerr   2021-07-29  316  static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
889b7da23abf92f Jeremy Kerr   2021-07-29  317  {
833ef3b91de692e Jeremy Kerr   2021-07-29  318  	struct net *net = dev_net(skb->dev);
833ef3b91de692e Jeremy Kerr   2021-07-29  319  	struct mctp_sk_key *key;
833ef3b91de692e Jeremy Kerr   2021-07-29  320  	struct mctp_sock *msk;
833ef3b91de692e Jeremy Kerr   2021-07-29  321  	struct mctp_hdr *mh;
4a992bbd3650947 Jeremy Kerr   2021-07-29  322  	unsigned long f;
4a992bbd3650947 Jeremy Kerr   2021-07-29  323  	u8 tag, flags;
4a992bbd3650947 Jeremy Kerr   2021-07-29  324  	int rc;
833ef3b91de692e Jeremy Kerr   2021-07-29  325  
833ef3b91de692e Jeremy Kerr   2021-07-29  326  	msk = NULL;
4a992bbd3650947 Jeremy Kerr   2021-07-29  327  	rc = -EINVAL;
833ef3b91de692e Jeremy Kerr   2021-07-29  328  
833ef3b91de692e Jeremy Kerr   2021-07-29  329  	/* we may be receiving a locally-routed packet; drop source sk
833ef3b91de692e Jeremy Kerr   2021-07-29  330  	 * accounting
833ef3b91de692e Jeremy Kerr   2021-07-29  331  	 */
833ef3b91de692e Jeremy Kerr   2021-07-29  332  	skb_orphan(skb);
833ef3b91de692e Jeremy Kerr   2021-07-29  333  
833ef3b91de692e Jeremy Kerr   2021-07-29  334  	/* ensure we have enough data for a header and a type */
833ef3b91de692e Jeremy Kerr   2021-07-29  335  	if (skb->len < sizeof(struct mctp_hdr) + 1)
4a992bbd3650947 Jeremy Kerr   2021-07-29  336  		goto out;
833ef3b91de692e Jeremy Kerr   2021-07-29  337  
833ef3b91de692e Jeremy Kerr   2021-07-29  338  	/* grab header, advance data ptr */
833ef3b91de692e Jeremy Kerr   2021-07-29  339  	mh = mctp_hdr(skb);
833ef3b91de692e Jeremy Kerr   2021-07-29  340  	skb_pull(skb, sizeof(struct mctp_hdr));
833ef3b91de692e Jeremy Kerr   2021-07-29  341  
833ef3b91de692e Jeremy Kerr   2021-07-29  342  	if (mh->ver != 1)
4a992bbd3650947 Jeremy Kerr   2021-07-29  343  		goto out;
833ef3b91de692e Jeremy Kerr   2021-07-29  344  
4a992bbd3650947 Jeremy Kerr   2021-07-29  345  	flags = mh->flags_seq_tag & (MCTP_HDR_FLAG_SOM | MCTP_HDR_FLAG_EOM);
4a992bbd3650947 Jeremy Kerr   2021-07-29  346  	tag = mh->flags_seq_tag & (MCTP_HDR_TAG_MASK | MCTP_HDR_FLAG_TO);
833ef3b91de692e Jeremy Kerr   2021-07-29  347  
833ef3b91de692e Jeremy Kerr   2021-07-29  348  	rcu_read_lock();
4a992bbd3650947 Jeremy Kerr   2021-07-29  349  
73c618456dc5cf2 Jeremy Kerr   2021-09-29  350  	/* lookup socket / reasm context, exactly matching (src,dest,tag).
73c618456dc5cf2 Jeremy Kerr   2021-09-29  351  	 * we hold a ref on the key, and key->lock held.
73c618456dc5cf2 Jeremy Kerr   2021-09-29  352  	 */
73c618456dc5cf2 Jeremy Kerr   2021-09-29  353  	key = mctp_lookup_key(net, skb, mh->src, &f);
833ef3b91de692e Jeremy Kerr   2021-07-29  354  
4a992bbd3650947 Jeremy Kerr   2021-07-29  355  	if (flags & MCTP_HDR_FLAG_SOM) {
4a992bbd3650947 Jeremy Kerr   2021-07-29  356  		if (key) {
4a992bbd3650947 Jeremy Kerr   2021-07-29  357  			msk = container_of(key->sk, struct mctp_sock, sk);
4a992bbd3650947 Jeremy Kerr   2021-07-29  358  		} else {
4a992bbd3650947 Jeremy Kerr   2021-07-29  359  			/* first response to a broadcast? do a more general
4a992bbd3650947 Jeremy Kerr   2021-07-29  360  			 * key lookup to find the socket, but don't use this
4a992bbd3650947 Jeremy Kerr   2021-07-29  361  			 * key for reassembly - we'll create a more specific
4a992bbd3650947 Jeremy Kerr   2021-07-29  362  			 * one for future packets if required (ie, !EOM).
4a992bbd3650947 Jeremy Kerr   2021-07-29  363  			 */
73c618456dc5cf2 Jeremy Kerr   2021-09-29  364  			key = mctp_lookup_key(net, skb, MCTP_ADDR_ANY, &f);
4a992bbd3650947 Jeremy Kerr   2021-07-29  365  			if (key) {
4a992bbd3650947 Jeremy Kerr   2021-07-29  366  				msk = container_of(key->sk,
4a992bbd3650947 Jeremy Kerr   2021-07-29  367  						   struct mctp_sock, sk);
73c618456dc5cf2 Jeremy Kerr   2021-09-29  368  				spin_unlock_irqrestore(&key->lock, f);
73c618456dc5cf2 Jeremy Kerr   2021-09-29  369  				mctp_key_unref(key);
4a992bbd3650947 Jeremy Kerr   2021-07-29  370  				key = NULL;
4a992bbd3650947 Jeremy Kerr   2021-07-29  371  			}
4a992bbd3650947 Jeremy Kerr   2021-07-29  372  		}
833ef3b91de692e Jeremy Kerr   2021-07-29  373  
4a992bbd3650947 Jeremy Kerr   2021-07-29  374  		if (!key && !msk && (tag & MCTP_HDR_FLAG_TO))
833ef3b91de692e Jeremy Kerr   2021-07-29  375  			msk = mctp_lookup_bind(net, skb);
833ef3b91de692e Jeremy Kerr   2021-07-29  376  
4a992bbd3650947 Jeremy Kerr   2021-07-29  377  		if (!msk) {
4a992bbd3650947 Jeremy Kerr   2021-07-29  378  			rc = -ENOENT;
4a992bbd3650947 Jeremy Kerr   2021-07-29  379  			goto out_unlock;
4a992bbd3650947 Jeremy Kerr   2021-07-29  380  		}
833ef3b91de692e Jeremy Kerr   2021-07-29  381  
4a992bbd3650947 Jeremy Kerr   2021-07-29  382  		/* single-packet message? deliver to socket, clean up any
4a992bbd3650947 Jeremy Kerr   2021-07-29  383  		 * pending key.
4a992bbd3650947 Jeremy Kerr   2021-07-29  384  		 */
4a992bbd3650947 Jeremy Kerr   2021-07-29  385  		if (flags & MCTP_HDR_FLAG_EOM) {
833ef3b91de692e Jeremy Kerr   2021-07-29  386  			sock_queue_rcv_skb(&msk->sk, skb);
4a992bbd3650947 Jeremy Kerr   2021-07-29  387  			if (key) {
4a992bbd3650947 Jeremy Kerr   2021-07-29  388  				/* we've hit a pending reassembly; not much we
4a992bbd3650947 Jeremy Kerr   2021-07-29  389  				 * can do but drop it
4a992bbd3650947 Jeremy Kerr   2021-07-29  390  				 */
a1d553f399d7457 Matt Johnston 2022-02-08  391  				__mctp_key_done_in(key, net, f,
4f9e1ba6de45aa8 Jeremy Kerr   2021-09-29  392  						   MCTP_TRACE_KEY_REPLIED);
73c618456dc5cf2 Jeremy Kerr   2021-09-29  393  				key = NULL;
4a992bbd3650947 Jeremy Kerr   2021-07-29  394  			}
4a992bbd3650947 Jeremy Kerr   2021-07-29  395  			rc = 0;
4a992bbd3650947 Jeremy Kerr   2021-07-29  396  			goto out_unlock;
4a992bbd3650947 Jeremy Kerr   2021-07-29  397  		}
833ef3b91de692e Jeremy Kerr   2021-07-29  398  
4a992bbd3650947 Jeremy Kerr   2021-07-29  399  		/* broadcast response or a bind() - create a key for further
4a992bbd3650947 Jeremy Kerr   2021-07-29  400  		 * packets for this message
4a992bbd3650947 Jeremy Kerr   2021-07-29  401  		 */
4a992bbd3650947 Jeremy Kerr   2021-07-29  402  		if (!key) {
4a992bbd3650947 Jeremy Kerr   2021-07-29  403  			key = mctp_key_alloc(msk, mh->dest, mh->src,
4a992bbd3650947 Jeremy Kerr   2021-07-29  404  					     tag, GFP_ATOMIC);
4a992bbd3650947 Jeremy Kerr   2021-07-29  405  			if (!key) {
4a992bbd3650947 Jeremy Kerr   2021-07-29  406  				rc = -ENOMEM;
4a992bbd3650947 Jeremy Kerr   2021-07-29  407  				goto out_unlock;
4a992bbd3650947 Jeremy Kerr   2021-07-29  408  			}
833ef3b91de692e Jeremy Kerr   2021-07-29  409  
73c618456dc5cf2 Jeremy Kerr   2021-09-29  410  			/* we can queue without the key lock here, as the
4a992bbd3650947 Jeremy Kerr   2021-07-29  411  			 * key isn't observable yet
4a992bbd3650947 Jeremy Kerr   2021-07-29  412  			 */
4a992bbd3650947 Jeremy Kerr   2021-07-29  413  			mctp_frag_queue(key, skb);
4a992bbd3650947 Jeremy Kerr   2021-07-29  414  
4a992bbd3650947 Jeremy Kerr   2021-07-29  415  			/* if the key_add fails, we've raced with another
4a992bbd3650947 Jeremy Kerr   2021-07-29  416  			 * SOM packet with the same src, dest and tag. There's
4a992bbd3650947 Jeremy Kerr   2021-07-29  417  			 * no way to distinguish future packets, so all we
4a992bbd3650947 Jeremy Kerr   2021-07-29  418  			 * can do is drop; we'll free the skb on exit from
4a992bbd3650947 Jeremy Kerr   2021-07-29  419  			 * this function.
4a992bbd3650947 Jeremy Kerr   2021-07-29  420  			 */
4a992bbd3650947 Jeremy Kerr   2021-07-29  421  			rc = mctp_key_add(key, msk);
4a992bbd3650947 Jeremy Kerr   2021-07-29  422  			if (rc)
4a992bbd3650947 Jeremy Kerr   2021-07-29  423  				kfree(key);
4a992bbd3650947 Jeremy Kerr   2021-07-29  424  
4f9e1ba6de45aa8 Jeremy Kerr   2021-09-29  425  			trace_mctp_key_acquire(key);
4f9e1ba6de45aa8 Jeremy Kerr   2021-09-29  426  
73c618456dc5cf2 Jeremy Kerr   2021-09-29  427  			/* we don't need to release key->lock on exit */
0b93aed2842d950 Matt Johnston 2021-10-14  428  			mctp_key_unref(key);
73c618456dc5cf2 Jeremy Kerr   2021-09-29  429  			key = NULL;
4a992bbd3650947 Jeremy Kerr   2021-07-29  430  
73c618456dc5cf2 Jeremy Kerr   2021-09-29  431  		} else {
4a992bbd3650947 Jeremy Kerr   2021-07-29  432  			if (key->reasm_head || key->reasm_dead) {
4a992bbd3650947 Jeremy Kerr   2021-07-29  433  				/* duplicate start? drop everything */
a1d553f399d7457 Matt Johnston 2022-02-08  434  				__mctp_key_done_in(key, net, f,
4f9e1ba6de45aa8 Jeremy Kerr   2021-09-29  435  						   MCTP_TRACE_KEY_INVALIDATED);
4a992bbd3650947 Jeremy Kerr   2021-07-29  436  				rc = -EEXIST;
73c618456dc5cf2 Jeremy Kerr   2021-09-29  437  				key = NULL;
4a992bbd3650947 Jeremy Kerr   2021-07-29  438  			} else {
4a992bbd3650947 Jeremy Kerr   2021-07-29  439  				rc = mctp_frag_queue(key, skb);
4a992bbd3650947 Jeremy Kerr   2021-07-29  440  			}
4a992bbd3650947 Jeremy Kerr   2021-07-29  441  		}
4a992bbd3650947 Jeremy Kerr   2021-07-29  442  
4a992bbd3650947 Jeremy Kerr   2021-07-29  443  	} else if (key) {
4a992bbd3650947 Jeremy Kerr   2021-07-29  444  		/* this packet continues a previous message; reassemble
4a992bbd3650947 Jeremy Kerr   2021-07-29  445  		 * using the message-specific key
4a992bbd3650947 Jeremy Kerr   2021-07-29  446  		 */
4a992bbd3650947 Jeremy Kerr   2021-07-29  447  
4a992bbd3650947 Jeremy Kerr   2021-07-29  448  		/* we need to be continuing an existing reassembly... */
4a992bbd3650947 Jeremy Kerr   2021-07-29  449  		if (!key->reasm_head)
4a992bbd3650947 Jeremy Kerr   2021-07-29  450  			rc = -EINVAL;
4a992bbd3650947 Jeremy Kerr   2021-07-29  451  		else
4a992bbd3650947 Jeremy Kerr   2021-07-29  452  			rc = mctp_frag_queue(key, skb);
4a992bbd3650947 Jeremy Kerr   2021-07-29  453  
4a992bbd3650947 Jeremy Kerr   2021-07-29  454  		/* end of message? deliver to socket, and we're done with
4a992bbd3650947 Jeremy Kerr   2021-07-29  455  		 * the reassembly/response key
4a992bbd3650947 Jeremy Kerr   2021-07-29  456  		 */
4a992bbd3650947 Jeremy Kerr   2021-07-29  457  		if (!rc && flags & MCTP_HDR_FLAG_EOM) {
a1d553f399d7457 Matt Johnston 2022-02-08 @458  			msk = container_of(key->sk, struct mctp_sock, sk);
4a992bbd3650947 Jeremy Kerr   2021-07-29  459  			sock_queue_rcv_skb(key->sk, key->reasm_head);
4a992bbd3650947 Jeremy Kerr   2021-07-29  460  			key->reasm_head = NULL;
a1d553f399d7457 Matt Johnston 2022-02-08  461  			__mctp_key_done_in(key, net, f, MCTP_TRACE_KEY_REPLIED);
73c618456dc5cf2 Jeremy Kerr   2021-09-29  462  			key = NULL;
4a992bbd3650947 Jeremy Kerr   2021-07-29  463  		}
4a992bbd3650947 Jeremy Kerr   2021-07-29  464  
4a992bbd3650947 Jeremy Kerr   2021-07-29  465  	} else {
4a992bbd3650947 Jeremy Kerr   2021-07-29  466  		/* not a start, no matching key */
4a992bbd3650947 Jeremy Kerr   2021-07-29  467  		rc = -ENOENT;
4a992bbd3650947 Jeremy Kerr   2021-07-29  468  	}
4a992bbd3650947 Jeremy Kerr   2021-07-29  469  
4a992bbd3650947 Jeremy Kerr   2021-07-29  470  out_unlock:
833ef3b91de692e Jeremy Kerr   2021-07-29  471  	rcu_read_unlock();
73c618456dc5cf2 Jeremy Kerr   2021-09-29  472  	if (key) {
73c618456dc5cf2 Jeremy Kerr   2021-09-29  473  		spin_unlock_irqrestore(&key->lock, f);
73c618456dc5cf2 Jeremy Kerr   2021-09-29  474  		mctp_key_unref(key);
73c618456dc5cf2 Jeremy Kerr   2021-09-29  475  	}
4a992bbd3650947 Jeremy Kerr   2021-07-29  476  out:
4a992bbd3650947 Jeremy Kerr   2021-07-29  477  	if (rc)
889b7da23abf92f Jeremy Kerr   2021-07-29  478  		kfree_skb(skb);
4a992bbd3650947 Jeremy Kerr   2021-07-29  479  	return rc;
4a992bbd3650947 Jeremy Kerr   2021-07-29  480  }
4a992bbd3650947 Jeremy Kerr   2021-07-29  481  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC,DROP}TAG ioctls for tag control
  2022-02-08  9:46 ` [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC,DROP}TAG ioctls for tag control Jeremy Kerr
@ 2022-02-08 16:10     ` kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-02-08 16:10 UTC (permalink / raw)
  To: Jeremy Kerr, netdev
  Cc: kbuild-all, Matt Johnston, Jakub Kicinski, Jonathan Corbet,
	Steven Rostedt, Ingo Molnar, linux-doc

Hi Jeremy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jeremy-Kerr/MCTP-tag-control-interface/20220208-195325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git c3e676b98326a419f30dd5d956c68fc33323f4fd
config: nios2-randconfig-r021-20220208 (https://download.01.org/0day-ci/archive/20220209/202202090043.BhR7muS4-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.2.0
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
        # https://github.com/0day-ci/linux/commit/a1d553f399d7457bd3e455cd3f5e10dddb4bc2bf
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jeremy-Kerr/MCTP-tag-control-interface/20220208-195325
        git checkout a1d553f399d7457bd3e455cd3f5e10dddb4bc2bf
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash kernel/ net/mctp/

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/mctp/route.c:660:21: warning: no previous prototype for 'mctp_lookup_prealloc_tag' [-Wmissing-prototypes]
     660 | struct mctp_sk_key *mctp_lookup_prealloc_tag(struct mctp_sock *msk,
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~


vim +/mctp_lookup_prealloc_tag +660 net/mctp/route.c

   659	
 > 660	struct mctp_sk_key *mctp_lookup_prealloc_tag(struct mctp_sock *msk,
   661						     mctp_eid_t daddr, u8 req_tag,
   662						     u8 *tagp)
   663	{
   664		struct net *net = sock_net(&msk->sk);
   665		struct netns_mctp *mns = &net->mctp;
   666		struct mctp_sk_key *key, *tmp;
   667		unsigned long flags;
   668	
   669		req_tag &= ~(MCTP_TAG_PREALLOC | MCTP_TAG_OWNER);
   670		key = NULL;
   671	
   672		spin_lock_irqsave(&mns->keys_lock, flags);
   673	
   674		hlist_for_each_entry(tmp, &mns->keys, hlist) {
   675			if (tmp->tag != req_tag)
   676				continue;
   677	
   678			if (!(tmp->peer_addr == daddr || tmp->peer_addr == MCTP_ADDR_ANY))
   679				continue;
   680	
   681			if (!tmp->manual_alloc)
   682				continue;
   683	
   684			spin_lock(&tmp->lock);
   685			if (tmp->valid) {
   686				key = tmp;
   687				refcount_inc(&key->refs);
   688				spin_unlock(&tmp->lock);
   689				break;
   690			}
   691			spin_unlock(&tmp->lock);
   692		}
   693		spin_unlock_irqrestore(&mns->keys_lock, flags);
   694	
   695		if (!key)
   696			return ERR_PTR(-ENOENT);
   697	
   698		if (tagp)
   699			*tagp = key->tag;
   700	
   701		return key;
   702	}
   703	

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC, DROP}TAG ioctls for tag control
@ 2022-02-08 16:10     ` kernel test robot
  0 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2022-02-08 16:10 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 3046 bytes --]

Hi Jeremy,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Jeremy-Kerr/MCTP-tag-control-interface/20220208-195325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git c3e676b98326a419f30dd5d956c68fc33323f4fd
config: nios2-randconfig-r021-20220208 (https://download.01.org/0day-ci/archive/20220209/202202090043.BhR7muS4-lkp(a)intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.2.0
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
        # https://github.com/0day-ci/linux/commit/a1d553f399d7457bd3e455cd3f5e10dddb4bc2bf
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Jeremy-Kerr/MCTP-tag-control-interface/20220208-195325
        git checkout a1d553f399d7457bd3e455cd3f5e10dddb4bc2bf
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=nios2 SHELL=/bin/bash kernel/ net/mctp/

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/mctp/route.c:660:21: warning: no previous prototype for 'mctp_lookup_prealloc_tag' [-Wmissing-prototypes]
     660 | struct mctp_sk_key *mctp_lookup_prealloc_tag(struct mctp_sock *msk,
         |                     ^~~~~~~~~~~~~~~~~~~~~~~~


vim +/mctp_lookup_prealloc_tag +660 net/mctp/route.c

   659	
 > 660	struct mctp_sk_key *mctp_lookup_prealloc_tag(struct mctp_sock *msk,
   661						     mctp_eid_t daddr, u8 req_tag,
   662						     u8 *tagp)
   663	{
   664		struct net *net = sock_net(&msk->sk);
   665		struct netns_mctp *mns = &net->mctp;
   666		struct mctp_sk_key *key, *tmp;
   667		unsigned long flags;
   668	
   669		req_tag &= ~(MCTP_TAG_PREALLOC | MCTP_TAG_OWNER);
   670		key = NULL;
   671	
   672		spin_lock_irqsave(&mns->keys_lock, flags);
   673	
   674		hlist_for_each_entry(tmp, &mns->keys, hlist) {
   675			if (tmp->tag != req_tag)
   676				continue;
   677	
   678			if (!(tmp->peer_addr == daddr || tmp->peer_addr == MCTP_ADDR_ANY))
   679				continue;
   680	
   681			if (!tmp->manual_alloc)
   682				continue;
   683	
   684			spin_lock(&tmp->lock);
   685			if (tmp->valid) {
   686				key = tmp;
   687				refcount_inc(&key->refs);
   688				spin_unlock(&tmp->lock);
   689				break;
   690			}
   691			spin_unlock(&tmp->lock);
   692		}
   693		spin_unlock_irqrestore(&mns->keys_lock, flags);
   694	
   695		if (!key)
   696			return ERR_PTR(-ENOENT);
   697	
   698		if (tagp)
   699			*tagp = key->tag;
   700	
   701		return key;
   702	}
   703	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC,DROP}TAG ioctls for tag control
  2022-02-08  9:46 [PATCH net-next 0/5] MCTP tag control interface Jeremy Kerr
@ 2022-02-08  9:46 ` Jeremy Kerr
  2022-02-08 16:10     ` [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC, DROP}TAG " kernel test robot
  0 siblings, 1 reply; 5+ messages in thread
From: Jeremy Kerr @ 2022-02-08  9:46 UTC (permalink / raw)
  To: netdev
  Cc: Matt Johnston, David S. Miller, Jakub Kicinski, Jonathan Corbet,
	Steven Rostedt, Ingo Molnar, linux-doc

From: Matt Johnston <matt@codeconstruct.com.au>

This change adds a couple of new ioctls for mctp sockets:
SIOCMCTPALLOCTAG and SIOCMCTPDROPTAG.  These ioctls provide facilities
for explicit allocation / release of tags, overriding the automatic
allocate-on-send/release-on-reply and timeout behaviours. This allows
userspace more control over messages that may not fit a simple
request/response model.

In order to indicate a pre-allocated tag to the sendmsg() syscall, we
introduce a new flag to the struct sockaddr_mctp.smctp_tag value:
MCTP_TAG_PREALLOC.

Additional changes from Jeremy Kerr <jk@codeconstruct.com.au>.

Signed-off-by: Matt Johnston <matt@codeconstruct.com.au>
Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au>
---
 Documentation/networking/mctp.rst |  48 ++++++++
 include/net/mctp.h                |  11 +-
 include/trace/events/mctp.h       |   5 +-
 include/uapi/linux/mctp.h         |  18 +++
 net/mctp/af_mctp.c                | 185 +++++++++++++++++++++++++-----
 net/mctp/route.c                  | 114 +++++++++++++-----
 6 files changed, 325 insertions(+), 56 deletions(-)

diff --git a/Documentation/networking/mctp.rst b/Documentation/networking/mctp.rst
index 46f74bffce0f..c628cb5406d2 100644
--- a/Documentation/networking/mctp.rst
+++ b/Documentation/networking/mctp.rst
@@ -212,6 +212,54 @@ remote address is already known, or the message does not require a reply.
 Like the send calls, sockets will only receive responses to requests they have
 sent (TO=1) and may only respond (TO=0) to requests they have received.
 
+``ioctl(SIOCMCTPALLOCTAG)`` and ``ioctl(SIOCMCTPDROPTAG)``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+These tags give applications more control over MCTP message tags, by allocating
+(and dropping) tag values explicitly, rather than the kernel automatically
+allocating a per-message tag at ``sendmsg()`` time.
+
+In general, you will only need to use these ioctls if your MCTP protocol does
+not fit the usual request/response model. For example, if you need to persist
+tags across multiple requests, or a request may generate more than one response.
+In these cases, the ioctls allow you to decouple the tag allocation (and
+release) from individual message send and receive operations.
+
+Both ioctls are passed a pointer to a ``struct mctp_ioc_tag_ctl``:
+
+.. code-block:: C
+
+    struct mctp_ioc_tag_ctl {
+        mctp_eid_t      peer_addr;
+        __u8		tag;
+        __u16   	flags;
+    };
+
+``SIOCMCTPALLOCTAG`` allocates a tag for a specific peer, which an application
+can use in future ``sendmsg()`` calls. The application populates the
+``peer_addr`` member with the remote EID. Other fields must be zero.
+
+On return, the ``tag`` member will be populated with the allocated tag value.
+The allocated tag will have the following tag bits set:
+
+ - ``MCTP_TAG_OWNER``: it only makes sense to allocate tags if you're the tag
+   owner
+
+ - ``MCTP_TAG_PREALLOC``: to indicate to ``sendmsg()`` that this is a
+   preallocated tag.
+
+ - ... and the actual tag value, within the least-significant three bits
+   (``MCTP_TAG_MASK``). Note that zero is a valid tag value.
+
+The tag value should be used as-is for the ``smctp_tag`` member of ``struct
+sockaddr_mctp``.
+
+``SIOCMCTPDROPTAG`` releases a tag that has been previously allocated by a
+``SIOCMCTPALLOCTAG`` ioctl. The ``peer_addr`` must be the same as used for the
+allocation, and the ``tag`` value must match exactly the tag returned from the
+allocation (including the ``MCTP_TAG_OWNER`` and ``MCTP_TAG_PREALLOC`` bits).
+The ``flags`` field must be zero.
+
 Kernel internals
 ================
 
diff --git a/include/net/mctp.h b/include/net/mctp.h
index 706d329dd8e8..e80a4baf8379 100644
--- a/include/net/mctp.h
+++ b/include/net/mctp.h
@@ -126,7 +126,7 @@ struct mctp_sock {
  */
 struct mctp_sk_key {
 	mctp_eid_t	peer_addr;
-	mctp_eid_t	local_addr;
+	mctp_eid_t	local_addr; /* MCTP_ADDR_ANY for local owned tags */
 	__u8		tag; /* incoming tag match; invert TO for local */
 
 	/* we hold a ref to sk when set */
@@ -163,6 +163,12 @@ struct mctp_sk_key {
 	 */
 	unsigned long	dev_flow_state;
 	struct mctp_dev	*dev;
+
+	/* a tag allocated with SIOCMCTPALLOCTAG ioctl will not expire
+	 * automatically on timeout or response, instead SIOCMCTPDROPTAG
+	 * is used.
+	 */
+	bool		manual_alloc;
 };
 
 struct mctp_skb_cb {
@@ -239,6 +245,9 @@ int mctp_local_output(struct sock *sk, struct mctp_route *rt,
 		      struct sk_buff *skb, mctp_eid_t daddr, u8 req_tag);
 
 void mctp_key_unref(struct mctp_sk_key *key);
+struct mctp_sk_key *mctp_alloc_local_tag(struct mctp_sock *msk,
+					 mctp_eid_t daddr, mctp_eid_t saddr,
+					 bool manual, u8 *tagp);
 
 /* routing <--> device interface */
 unsigned int mctp_default_net(struct net *net);
diff --git a/include/trace/events/mctp.h b/include/trace/events/mctp.h
index 175b057c507f..165cf25f77a7 100644
--- a/include/trace/events/mctp.h
+++ b/include/trace/events/mctp.h
@@ -15,6 +15,7 @@ enum {
 	MCTP_TRACE_KEY_REPLIED,
 	MCTP_TRACE_KEY_INVALIDATED,
 	MCTP_TRACE_KEY_CLOSED,
+	MCTP_TRACE_KEY_DROPPED,
 };
 #endif /* __TRACE_MCTP_ENUMS */
 
@@ -22,6 +23,7 @@ TRACE_DEFINE_ENUM(MCTP_TRACE_KEY_TIMEOUT);
 TRACE_DEFINE_ENUM(MCTP_TRACE_KEY_REPLIED);
 TRACE_DEFINE_ENUM(MCTP_TRACE_KEY_INVALIDATED);
 TRACE_DEFINE_ENUM(MCTP_TRACE_KEY_CLOSED);
+TRACE_DEFINE_ENUM(MCTP_TRACE_KEY_DROPPED);
 
 TRACE_EVENT(mctp_key_acquire,
 	TP_PROTO(const struct mctp_sk_key *key),
@@ -66,7 +68,8 @@ TRACE_EVENT(mctp_key_release,
 				 { MCTP_TRACE_KEY_TIMEOUT, "timeout" },
 				 { MCTP_TRACE_KEY_REPLIED, "replied" },
 				 { MCTP_TRACE_KEY_INVALIDATED, "invalidated" },
-				 { MCTP_TRACE_KEY_CLOSED, "closed" })
+				 { MCTP_TRACE_KEY_CLOSED, "closed" },
+				 { MCTP_TRACE_KEY_DROPPED, "dropped" })
 	)
 );
 
diff --git a/include/uapi/linux/mctp.h b/include/uapi/linux/mctp.h
index 07b0318716fc..154ab56651f1 100644
--- a/include/uapi/linux/mctp.h
+++ b/include/uapi/linux/mctp.h
@@ -44,7 +44,25 @@ struct sockaddr_mctp_ext {
 
 #define MCTP_TAG_MASK		0x07
 #define MCTP_TAG_OWNER		0x08
+#define MCTP_TAG_PREALLOC	0x10
 
 #define MCTP_OPT_ADDR_EXT	1
 
+#define SIOCMCTPALLOCTAG	(SIOCPROTOPRIVATE + 0)
+#define SIOCMCTPDROPTAG		(SIOCPROTOPRIVATE + 1)
+
+struct mctp_ioc_tag_ctl {
+	mctp_eid_t	peer_addr;
+
+	/* For SIOCMCTPALLOCTAG: must be passed as zero, kernel will
+	 * populate with the allocated tag value. Returned tag value will
+	 * always have TO and PREALLOC set.
+	 *
+	 * For SIOCMCTPDROPTAG: userspace provides tag value to drop, from
+	 * a prior SIOCMCTPALLOCTAG call (and so must have TO and PREALLOC set).
+	 */
+	__u8		tag;
+	__u16		flags;
+};
+
 #endif /* __UAPI_MCTP_H */
diff --git a/net/mctp/af_mctp.c b/net/mctp/af_mctp.c
index c921de63b494..769fca872aa1 100644
--- a/net/mctp/af_mctp.c
+++ b/net/mctp/af_mctp.c
@@ -6,6 +6,7 @@
  * Copyright (c) 2021 Google
  */
 
+#include <linux/compat.h>
 #include <linux/if_arp.h>
 #include <linux/net.h>
 #include <linux/mctp.h>
@@ -21,6 +22,8 @@
 
 /* socket implementation */
 
+static void mctp_sk_expire_keys(struct timer_list *timer);
+
 static int mctp_release(struct socket *sock)
 {
 	struct sock *sk = sock->sk;
@@ -99,13 +102,20 @@ static int mctp_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
 	struct sk_buff *skb;
 
 	if (addr) {
+		const u8 tagbits = MCTP_TAG_MASK | MCTP_TAG_OWNER |
+			MCTP_TAG_PREALLOC;
+
 		if (addrlen < sizeof(struct sockaddr_mctp))
 			return -EINVAL;
 		if (addr->smctp_family != AF_MCTP)
 			return -EINVAL;
 		if (!mctp_sockaddr_is_ok(addr))
 			return -EINVAL;
-		if (addr->smctp_tag & ~(MCTP_TAG_MASK | MCTP_TAG_OWNER))
+		if (addr->smctp_tag & ~tagbits)
+			return -EINVAL;
+		/* can't preallocate a non-owned tag */
+		if (addr->smctp_tag & MCTP_TAG_PREALLOC &&
+		    !(addr->smctp_tag & MCTP_TAG_OWNER))
 			return -EINVAL;
 
 	} else {
@@ -248,6 +258,32 @@ static int mctp_recvmsg(struct socket *sock, struct msghdr *msg, size_t len,
 	return rc;
 }
 
+/* We're done with the key; invalidate, stop reassembly, and remove from lists.
+ */
+static void __mctp_key_remove(struct mctp_sk_key *key, struct net *net,
+			      unsigned long flags, unsigned long reason)
+__releases(&key->lock)
+__must_hold(&net->mctp.keys_lock)
+{
+	struct sk_buff *skb;
+
+	trace_mctp_key_release(key, reason);
+	skb = key->reasm_head;
+	key->reasm_head = NULL;
+	key->reasm_dead = true;
+	key->valid = false;
+	mctp_dev_release_key(key->dev, key);
+	spin_unlock_irqrestore(&key->lock, flags);
+
+	hlist_del(&key->hlist);
+	hlist_del(&key->sklist);
+
+	/* unref for the lists */
+	mctp_key_unref(key);
+
+	kfree_skb(skb);
+}
+
 static int mctp_setsockopt(struct socket *sock, int level, int optname,
 			   sockptr_t optval, unsigned int optlen)
 {
@@ -293,6 +329,114 @@ static int mctp_getsockopt(struct socket *sock, int level, int optname,
 	return -EINVAL;
 }
 
+static int mctp_ioctl_alloctag(struct mctp_sock *msk, unsigned long arg)
+{
+	struct net *net = sock_net(&msk->sk);
+	struct mctp_sk_key *key = NULL;
+	struct mctp_ioc_tag_ctl ctl;
+	unsigned long flags;
+	u8 tag;
+
+	if (copy_from_user(&ctl, (void __user *)arg, sizeof(ctl)))
+		return -EFAULT;
+
+	if (ctl.tag)
+		return -EINVAL;
+
+	if (ctl.flags)
+		return -EINVAL;
+
+	key = mctp_alloc_local_tag(msk, ctl.peer_addr, MCTP_ADDR_ANY,
+				   true, &tag);
+	if (IS_ERR(key))
+		return PTR_ERR(key);
+
+	ctl.tag = tag | MCTP_TAG_OWNER | MCTP_TAG_PREALLOC;
+	if (copy_to_user((void __user *)arg, &ctl, sizeof(ctl))) {
+		spin_lock_irqsave(&key->lock, flags);
+		__mctp_key_remove(key, net, flags, MCTP_TRACE_KEY_DROPPED);
+		mctp_key_unref(key);
+		return -EFAULT;
+	}
+
+	mctp_key_unref(key);
+	return 0;
+}
+
+static int mctp_ioctl_droptag(struct mctp_sock *msk, unsigned long arg)
+{
+	struct net *net = sock_net(&msk->sk);
+	struct mctp_ioc_tag_ctl ctl;
+	unsigned long flags, fl2;
+	struct mctp_sk_key *key;
+	struct hlist_node *tmp;
+	int rc;
+	u8 tag;
+
+	if (copy_from_user(&ctl, (void __user *)arg, sizeof(ctl)))
+		return -EFAULT;
+
+	if (ctl.flags)
+		return -EINVAL;
+
+	/* Must be a local tag, TO set, preallocated */
+	if ((ctl.tag & ~MCTP_TAG_MASK) != (MCTP_TAG_OWNER | MCTP_TAG_PREALLOC))
+		return -EINVAL;
+
+	tag = ctl.tag & MCTP_TAG_MASK;
+	rc = -EINVAL;
+
+	spin_lock_irqsave(&net->mctp.keys_lock, flags);
+	hlist_for_each_entry_safe(key, tmp, &msk->keys, sklist) {
+		/* we do an irqsave here, even though we know the irq state,
+		 * so we have the flags to pass to __mctp_key_remove
+		 */
+		spin_lock_irqsave(&key->lock, fl2);
+		if (key->manual_alloc &&
+		    ctl.peer_addr == key->peer_addr &&
+		    tag == key->tag) {
+			__mctp_key_remove(key, net, fl2, MCTP_TRACE_KEY_DROPPED);
+			rc = 0;
+		} else {
+			spin_unlock_irqrestore(&key->lock, fl2);
+		}
+	}
+	spin_unlock_irqrestore(&net->mctp.keys_lock, flags);
+
+	return rc;
+}
+
+static int mctp_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
+{
+	struct mctp_sock *msk = container_of(sock->sk, struct mctp_sock, sk);
+
+	switch (cmd) {
+	case SIOCMCTPALLOCTAG:
+		return mctp_ioctl_alloctag(msk, arg);
+	case SIOCMCTPDROPTAG:
+		return mctp_ioctl_droptag(msk, arg);
+	}
+
+	return -EINVAL;
+}
+
+#ifdef CONFIG_COMPAT
+static int mctp_compat_ioctl(struct socket *sock, unsigned int cmd,
+			     unsigned long arg)
+{
+	void __user *argp = compat_ptr(arg);
+
+	switch (cmd) {
+	/* These have compatible ptr layouts */
+	case SIOCMCTPALLOCTAG:
+	case SIOCMCTPDROPTAG:
+		return mctp_ioctl(sock, cmd, (unsigned long)argp);
+	}
+
+	return -ENOIOCTLCMD;
+}
+#endif
+
 static const struct proto_ops mctp_dgram_ops = {
 	.family		= PF_MCTP,
 	.release	= mctp_release,
@@ -302,7 +446,7 @@ static const struct proto_ops mctp_dgram_ops = {
 	.accept		= sock_no_accept,
 	.getname	= sock_no_getname,
 	.poll		= datagram_poll,
-	.ioctl		= sock_no_ioctl,
+	.ioctl		= mctp_ioctl,
 	.gettstamp	= sock_gettstamp,
 	.listen		= sock_no_listen,
 	.shutdown	= sock_no_shutdown,
@@ -312,6 +456,9 @@ static const struct proto_ops mctp_dgram_ops = {
 	.recvmsg	= mctp_recvmsg,
 	.mmap		= sock_no_mmap,
 	.sendpage	= sock_no_sendpage,
+#ifdef CONFIG_COMPAT
+	.compat_ioctl	= mctp_compat_ioctl,
+#endif
 };
 
 static void mctp_sk_expire_keys(struct timer_list *timer)
@@ -319,7 +466,7 @@ static void mctp_sk_expire_keys(struct timer_list *timer)
 	struct mctp_sock *msk = container_of(timer, struct mctp_sock,
 					     key_expiry);
 	struct net *net = sock_net(&msk->sk);
-	unsigned long next_expiry, flags;
+	unsigned long next_expiry, flags, fl2;
 	struct mctp_sk_key *key;
 	struct hlist_node *tmp;
 	bool next_expiry_valid = false;
@@ -327,15 +474,13 @@ static void mctp_sk_expire_keys(struct timer_list *timer)
 	spin_lock_irqsave(&net->mctp.keys_lock, flags);
 
 	hlist_for_each_entry_safe(key, tmp, &msk->keys, sklist) {
-		spin_lock(&key->lock);
+		/* don't expire. manual_alloc is immutable, no locking required */
+		if (key->manual_alloc)
+			continue;
 
+		spin_lock_irqsave(&key->lock, fl2);
 		if (!time_after_eq(key->expiry, jiffies)) {
-			trace_mctp_key_release(key, MCTP_TRACE_KEY_TIMEOUT);
-			key->valid = false;
-			hlist_del_rcu(&key->hlist);
-			hlist_del_rcu(&key->sklist);
-			spin_unlock(&key->lock);
-			mctp_key_unref(key);
+			__mctp_key_remove(key, net, fl2, MCTP_TRACE_KEY_TIMEOUT);
 			continue;
 		}
 
@@ -346,7 +491,7 @@ static void mctp_sk_expire_keys(struct timer_list *timer)
 			next_expiry = key->expiry;
 			next_expiry_valid = true;
 		}
-		spin_unlock(&key->lock);
+		spin_unlock_irqrestore(&key->lock, fl2);
 	}
 
 	spin_unlock_irqrestore(&net->mctp.keys_lock, flags);
@@ -387,9 +532,9 @@ static void mctp_sk_unhash(struct sock *sk)
 {
 	struct mctp_sock *msk = container_of(sk, struct mctp_sock, sk);
 	struct net *net = sock_net(sk);
+	unsigned long flags, fl2;
 	struct mctp_sk_key *key;
 	struct hlist_node *tmp;
-	unsigned long flags;
 
 	/* remove from any type-based binds */
 	mutex_lock(&net->mctp.bind_lock);
@@ -399,20 +544,8 @@ static void mctp_sk_unhash(struct sock *sk)
 	/* remove tag allocations */
 	spin_lock_irqsave(&net->mctp.keys_lock, flags);
 	hlist_for_each_entry_safe(key, tmp, &msk->keys, sklist) {
-		hlist_del(&key->sklist);
-		hlist_del(&key->hlist);
-
-		trace_mctp_key_release(key, MCTP_TRACE_KEY_CLOSED);
-
-		spin_lock(&key->lock);
-		kfree_skb(key->reasm_head);
-		key->reasm_head = NULL;
-		key->reasm_dead = true;
-		key->valid = false;
-		spin_unlock(&key->lock);
-
-		/* key is no longer on the lookup lists, unref */
-		mctp_key_unref(key);
+		spin_lock_irqsave(&key->lock, fl2);
+		__mctp_key_remove(key, net, fl2, MCTP_TRACE_KEY_CLOSED);
 	}
 	spin_unlock_irqrestore(&net->mctp.keys_lock, flags);
 }
diff --git a/net/mctp/route.c b/net/mctp/route.c
index 35f72e99e188..2e79d5227dc8 100644
--- a/net/mctp/route.c
+++ b/net/mctp/route.c
@@ -203,29 +203,38 @@ static int mctp_key_add(struct mctp_sk_key *key, struct mctp_sock *msk)
 	return rc;
 }
 
-/* We're done with the key; unset valid and remove from lists. There may still
- * be outstanding refs on the key though...
+/* Helper for mctp_route_input().
+ * We're done with the key; unlock and unref the key.
+ * For the usual case of automatic expiry we remove the key from lists.
+ * In the case that manual allocation is set on a key we release the lock
+ * and local ref, reset reassembly, but don't remove from lists.
  */
-static void __mctp_key_unlock_drop(struct mctp_sk_key *key, struct net *net,
-				   unsigned long flags)
-	__releases(&key->lock)
+static void __mctp_key_done_in(struct mctp_sk_key *key, struct net *net,
+			       unsigned long flags, unsigned long reason)
+__releases(&key->lock)
 {
 	struct sk_buff *skb;
 
+	trace_mctp_key_release(key, reason);
 	skb = key->reasm_head;
 	key->reasm_head = NULL;
-	key->reasm_dead = true;
-	key->valid = false;
-	mctp_dev_release_key(key->dev, key);
+
+	if (!key->manual_alloc) {
+		key->reasm_dead = true;
+		key->valid = false;
+		mctp_dev_release_key(key->dev, key);
+	}
 	spin_unlock_irqrestore(&key->lock, flags);
 
-	spin_lock_irqsave(&net->mctp.keys_lock, flags);
-	hlist_del(&key->hlist);
-	hlist_del(&key->sklist);
-	spin_unlock_irqrestore(&net->mctp.keys_lock, flags);
+	if (!key->manual_alloc) {
+		spin_lock_irqsave(&net->mctp.keys_lock, flags);
+		hlist_del(&key->hlist);
+		hlist_del(&key->sklist);
+		spin_unlock_irqrestore(&net->mctp.keys_lock, flags);
 
-	/* one unref for the lists */
-	mctp_key_unref(key);
+		/* unref for the lists */
+		mctp_key_unref(key);
+	}
 
 	/* and one for the local reference */
 	mctp_key_unref(key);
@@ -379,9 +388,8 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
 				/* we've hit a pending reassembly; not much we
 				 * can do but drop it
 				 */
-				trace_mctp_key_release(key,
-						       MCTP_TRACE_KEY_REPLIED);
-				__mctp_key_unlock_drop(key, net, f);
+				__mctp_key_done_in(key, net, f,
+						   MCTP_TRACE_KEY_REPLIED);
 				key = NULL;
 			}
 			rc = 0;
@@ -423,9 +431,8 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
 		} else {
 			if (key->reasm_head || key->reasm_dead) {
 				/* duplicate start? drop everything */
-				trace_mctp_key_release(key,
-						       MCTP_TRACE_KEY_INVALIDATED);
-				__mctp_key_unlock_drop(key, net, f);
+				__mctp_key_done_in(key, net, f,
+						   MCTP_TRACE_KEY_INVALIDATED);
 				rc = -EEXIST;
 				key = NULL;
 			} else {
@@ -448,10 +455,10 @@ static int mctp_route_input(struct mctp_route *route, struct sk_buff *skb)
 		 * the reassembly/response key
 		 */
 		if (!rc && flags & MCTP_HDR_FLAG_EOM) {
+			msk = container_of(key->sk, struct mctp_sock, sk);
 			sock_queue_rcv_skb(key->sk, key->reasm_head);
 			key->reasm_head = NULL;
-			trace_mctp_key_release(key, MCTP_TRACE_KEY_REPLIED);
-			__mctp_key_unlock_drop(key, net, f);
+			__mctp_key_done_in(key, net, f, MCTP_TRACE_KEY_REPLIED);
 			key = NULL;
 		}
 
@@ -579,9 +586,9 @@ static void mctp_reserve_tag(struct net *net, struct mctp_sk_key *key,
 /* Allocate a locally-owned tag value for (saddr, daddr), and reserve
  * it for the socket msk
  */
-static struct mctp_sk_key *mctp_alloc_local_tag(struct mctp_sock *msk,
-						mctp_eid_t saddr,
-						mctp_eid_t daddr, u8 *tagp)
+struct mctp_sk_key *mctp_alloc_local_tag(struct mctp_sock *msk,
+					 mctp_eid_t daddr, mctp_eid_t saddr,
+					 bool manual, u8 *tagp)
 {
 	struct net *net = sock_net(&msk->sk);
 	struct netns_mctp *mns = &net->mctp;
@@ -636,6 +643,7 @@ static struct mctp_sk_key *mctp_alloc_local_tag(struct mctp_sock *msk,
 		mctp_reserve_tag(net, key, msk);
 		trace_mctp_key_acquire(key);
 
+		key->manual_alloc = manual;
 		*tagp = key->tag;
 	}
 
@@ -649,6 +657,50 @@ static struct mctp_sk_key *mctp_alloc_local_tag(struct mctp_sock *msk,
 	return key;
 }
 
+struct mctp_sk_key *mctp_lookup_prealloc_tag(struct mctp_sock *msk,
+					     mctp_eid_t daddr, u8 req_tag,
+					     u8 *tagp)
+{
+	struct net *net = sock_net(&msk->sk);
+	struct netns_mctp *mns = &net->mctp;
+	struct mctp_sk_key *key, *tmp;
+	unsigned long flags;
+
+	req_tag &= ~(MCTP_TAG_PREALLOC | MCTP_TAG_OWNER);
+	key = NULL;
+
+	spin_lock_irqsave(&mns->keys_lock, flags);
+
+	hlist_for_each_entry(tmp, &mns->keys, hlist) {
+		if (tmp->tag != req_tag)
+			continue;
+
+		if (!(tmp->peer_addr == daddr || tmp->peer_addr == MCTP_ADDR_ANY))
+			continue;
+
+		if (!tmp->manual_alloc)
+			continue;
+
+		spin_lock(&tmp->lock);
+		if (tmp->valid) {
+			key = tmp;
+			refcount_inc(&key->refs);
+			spin_unlock(&tmp->lock);
+			break;
+		}
+		spin_unlock(&tmp->lock);
+	}
+	spin_unlock_irqrestore(&mns->keys_lock, flags);
+
+	if (!key)
+		return ERR_PTR(-ENOENT);
+
+	if (tagp)
+		*tagp = key->tag;
+
+	return key;
+}
+
 /* routing lookups */
 static bool mctp_rt_match_eid(struct mctp_route *rt,
 			      unsigned int net, mctp_eid_t eid)
@@ -843,8 +895,14 @@ int mctp_local_output(struct sock *sk, struct mctp_route *rt,
 	if (rc)
 		goto out_release;
 
-	if (req_tag & MCTP_HDR_FLAG_TO) {
-		key = mctp_alloc_local_tag(msk, saddr, daddr, &tag);
+	if (req_tag & MCTP_TAG_OWNER) {
+		if (req_tag & MCTP_TAG_PREALLOC)
+			key = mctp_lookup_prealloc_tag(msk, daddr,
+						       req_tag, &tag);
+		else
+			key = mctp_alloc_local_tag(msk, daddr, saddr,
+						   false, &tag);
+
 		if (IS_ERR(key)) {
 			rc = PTR_ERR(key);
 			goto out_release;
@@ -855,7 +913,7 @@ int mctp_local_output(struct sock *sk, struct mctp_route *rt,
 		tag |= MCTP_HDR_FLAG_TO;
 	} else {
 		key = NULL;
-		tag = req_tag;
+		tag = req_tag & MCTP_TAG_MASK;
 	}
 
 	skb->protocol = htons(ETH_P_MCTP);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-02-13  1:46 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-13  1:46 [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC, DROP}TAG ioctls for tag control kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2022-02-09 17:43 kernel test robot
2022-02-08  9:46 [PATCH net-next 0/5] MCTP tag control interface Jeremy Kerr
2022-02-08  9:46 ` [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC,DROP}TAG ioctls for tag control Jeremy Kerr
2022-02-08 16:10   ` kernel test robot
2022-02-08 16:10     ` [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC, DROP}TAG " kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.