* 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* [PATCH net-next 0/5] MCTP tag control interface
@ 2022-02-08 9:46 Jeremy Kerr
2022-02-08 9:46 ` [PATCH net-next 5/5] mctp: Add SIOCMCTP{ALLOC,DROP}TAG ioctls for tag control Jeremy Kerr
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
This series implements a small interface for userspace-controlled
message tag allocation for the MCTP protocol. Rather than leaving the
kernel to allocate per-message tag values, userspace can explicitly
allocate (and release) message tags through two new ioctls:
SIOCMCTPALLOCTAG and SIOCMCTPDROPTAG.
In order to do this, we first introduce some minor changes to the tag
handling, including a couple of new tests for the route input paths.
As always, any comments/queries/etc are most welcome.
Cheers,
Jeremy
---
Jeremy Kerr (4):
mctp: tests: Rename FL_T macro to FL_TO
mctp: tests: Add key state tests
mctp: Add helper for address match checking
mctp: Allow keys matching any local address
Matt Johnston (1):
mctp: Add SIOCMCTP{ALLOC,DROP}TAG ioctls for tag control
Documentation/networking/mctp.rst | 48 ++++++++
include/net/mctp.h | 16 ++-
include/trace/events/mctp.h | 5 +-
include/uapi/linux/mctp.h | 18 +++
net/mctp/af_mctp.c | 185 +++++++++++++++++++++++++-----
net/mctp/route.c | 124 ++++++++++++++------
net/mctp/test/route-test.c | 158 ++++++++++++++++++++++++-
7 files changed, 486 insertions(+), 68 deletions(-)
--
2.34.1
^ 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
* 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
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.