Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-Wvarargs in security/keys/trusted.c #41

Closed
nickdesaulniers opened this issue Sep 11, 2018 · 12 comments
Closed

-Wvarargs in security/keys/trusted.c #41

nickdesaulniers opened this issue Sep 11, 2018 · 12 comments
Assignees
Labels
-Wvarargs [BUG] linux A bug that should be fixed in the mainline kernel. [FIXED][LINUX] 5.1 This bug was fixed in Linux 5.1

Comments

@nickdesaulniers
Copy link
Member

  CC      security/keys/trusted.o
security/keys/trusted.c:146:17: warning: passing an object that undergoes default
      argument promotion to 'va_start' has undefined behavior [-Wvarargs]
        va_start(argp, h3);
                       ^
security/keys/trusted.c:126:37: note: parameter of type 'unsigned char' is declared
      here
                        unsigned char *h2, unsigned char h3, ...)
                                                         ^

guessing this requires CONFIG_TRUSTED_KEYS=y

@nickdesaulniers nickdesaulniers added [BUG] Untriaged Something isn't working low priority This bug is not critical and not a priority labels Sep 11, 2018
@nickdesaulniers nickdesaulniers added this to the ARCH=arm64 allyesconfig milestone Sep 11, 2018
@nathanchance
Copy link
Member

Okay I really went down the rabbit hole on this one and I apologize if any of my terminology is incorrect.

I believe Clang is complaining because varargs' default argument promotion for type char is unsigned char *. I did a somewhat thorough sweep of the tree and pretty much all va_start instances had the second parameter of type unsigned char *. This seems to make sense according to the C89 standard, section 4.8: http://port70.net/~nsz/c/c89/c89-draft.txt.

Now, that leads to two solutions...

Solution 1:

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index b69d3b1777c2..24fe2e81f660 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -123,13 +123,12 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
  */
 static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
                        unsigned int keylen, unsigned char *h1,
-                       unsigned char *h2, unsigned char h3, ...)
+                       unsigned char *h2, ...)
 {
        unsigned char paramdigest[SHA1_DIGEST_SIZE];
        struct sdesc *sdesc;
        unsigned int dlen;
        unsigned char *data;
-       unsigned char c;
        int ret;     
        va_list argp;

@@ -139,11 +138,10 @@ static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
                return PTR_ERR(sdesc); 
        }

-       c = h3;
        ret = crypto_shash_init(&sdesc->shash);
        if (ret < 0)
                goto out;
-       va_start(argp, h3);
+       va_start(argp, h2);
        for (;;) {
                dlen = va_arg(argp, unsigned int);
                if (dlen == 0)
@@ -163,7 +161,7 @@ static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
        if (!ret)
                ret = TSS_rawhmac(digest, key, keylen, SHA1_DIGEST_SIZE,
                                  paramdigest, TPM_NONCE_SIZE, h1,
-                                 TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
+                                 TPM_NONCE_SIZE, h2, 1, 0, 0);
 out:
        kzfree(sdesc);
        return ret;
@@ -508,7 +506,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
        if (pcrinfosize == 0) {
                /* no pcr info specified */
                ret = TSS_authhmac(td->pubauth, sess.secret, SHA1_DIGEST_SIZE,
-                                  sess.enonce, td->nonceodd, cont,
+                                  sess.enonce, td->nonceodd,
                                   sizeof(uint32_t), &ordinal, SHA1_DIGEST_SIZE,
                                   td->encauth, sizeof(uint32_t), &pcrsize,
                                   sizeof(uint32_t), &datsize, datalen, data, 0,
@@ -516,7 +514,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
        } else {
                /* pcr info specified */
                ret = TSS_authhmac(td->pubauth, sess.secret, SHA1_DIGEST_SIZE,
-                                  sess.enonce, td->nonceodd, cont,
+                                  sess.enonce, td->nonceodd,
                                   sizeof(uint32_t), &ordinal, SHA1_DIGEST_SIZE,
                                   td->encauth, sizeof(uint32_t), &pcrsize,
                                   pcrinfosize, pcrinfo, sizeof(uint32_t),
@@ -608,12 +606,12 @@ static int tpm_unseal(struct tpm_buf *tb,
                return ret;
        }
        ret = TSS_authhmac(authdata1, keyauth, TPM_NONCE_SIZE,
-                          enonce1, nonceodd, cont, sizeof(uint32_t),
+                          enonce1, nonceodd, sizeof(uint32_t),
                           &ordinal, bloblen, blob, 0, 0);
        if (ret < 0)
                return ret;
        ret = TSS_authhmac(authdata2, blobauth, TPM_NONCE_SIZE,
-                          enonce2, nonceodd, cont, sizeof(uint32_t),
+                          enonce2, nonceodd, sizeof(uint32_t),
                           &ordinal, bloblen, blob, 0, 0);
        if (ret < 0)
                return ret;

Solution 2:

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index b69d3b1777c2..ec47c705bb42 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -123,13 +123,12 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
  */
 static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
                        unsigned int keylen, unsigned char *h1,
-                       unsigned char *h2, unsigned char h3, ...)
+                       unsigned char *h2, unsigned char *h3, ...)
 {
        unsigned char paramdigest[SHA1_DIGEST_SIZE];
        struct sdesc *sdesc;
        unsigned int dlen;
-       unsigned char *data;
-       unsigned char c;
+       unsigned char *data, *c;
        int ret;
        va_list argp;
 
@@ -163,7 +162,7 @@ static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
        if (!ret)
                ret = TSS_rawhmac(digest, key, keylen, SHA1_DIGEST_SIZE,
                                  paramdigest, TPM_NONCE_SIZE, h1,
-                                 TPM_NONCE_SIZE, h2, 1, &c, 0, 0);
+                                 TPM_NONCE_SIZE, h2, 1, c, 0, 0);
 out:
        kzfree(sdesc);
        return ret;
@@ -508,7 +507,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
        if (pcrinfosize == 0) {
                /* no pcr info specified */
                ret = TSS_authhmac(td->pubauth, sess.secret, SHA1_DIGEST_SIZE,
-                                  sess.enonce, td->nonceodd, cont,
+                                  sess.enonce, td->nonceodd, &cont,
                                   sizeof(uint32_t), &ordinal, SHA1_DIGEST_SIZE,
                                   td->encauth, sizeof(uint32_t), &pcrsize,
                                   sizeof(uint32_t), &datsize, datalen, data, 0,
@@ -516,7 +515,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype,
        } else {
                /* pcr info specified */
                ret = TSS_authhmac(td->pubauth, sess.secret, SHA1_DIGEST_SIZE,
-                                  sess.enonce, td->nonceodd, cont,
+                                  sess.enonce, td->nonceodd, &cont,
                                   sizeof(uint32_t), &ordinal, SHA1_DIGEST_SIZE,
                                   td->encauth, sizeof(uint32_t), &pcrsize,
                                   pcrinfosize, pcrinfo, sizeof(uint32_t),
@@ -608,12 +607,12 @@ static int tpm_unseal(struct tpm_buf *tb,
                return ret;
        }
        ret = TSS_authhmac(authdata1, keyauth, TPM_NONCE_SIZE,
-                          enonce1, nonceodd, cont, sizeof(uint32_t),
+                          enonce1, nonceodd, &cont, sizeof(uint32_t),
                           &ordinal, bloblen, blob, 0, 0);
        if (ret < 0)
                return ret;
        ret = TSS_authhmac(authdata2, blobauth, TPM_NONCE_SIZE,
-                          enonce2, nonceodd, cont, sizeof(uint32_t),
+                          enonce2, nonceodd, &cont, sizeof(uint32_t),
                           &ordinal, bloblen, blob, 0, 0);
        if (ret < 0)
                return ret;

I came up with solution 1 because if I am following the code correctly (super possible I'm not), cont is passed by value, rather than reference, to TSS_authhmac and TSS_authhmac passes the local variable c by value to TSS_rawhmac where it isn't used (since data = va_arg(argp, unsigned char *); is going to use h1 if I am reading the man page correctly [and understanding the flow of the code]).

Neither solution may be correct and I could just be rambling at this point lol. If nobody has any idea, I'll reach out to the maintainers in a couple days with the first part of my analysis and ask what was intended.

@nickdesaulniers
Copy link
Member Author

I'm not sure; let's nail down the easier issues first then revisit.

@nathanchance
Copy link
Member

Completely fine with me, thanks

@nickdesaulniers
Copy link
Member Author

The C Standard, subclause 7.16.1.4, paragraph 4 [ISO/IEC 9899:2011] (and C90: 4.8.1.1):
The parameter parmN is the identifier of the rightmost parameter in the variable parameter list in the function definition (the one just before the ...). If the parameter parmN is declared with the register storage class, with a function or array type, or with a type that is not compatible with the type that results after application of the default argument promotions, the behavior is undefined.

believe Clang is complaining because varargs' default argument promotion for type char is unsigned char *.

I would have expected int.

The man page for va_start only mentions the promotion behavior for va_arg(), not va_start().

I think

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index b69d3b1777c2..8e532873429c 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -123,7 +123,7 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key,
  */
 static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
 			unsigned int keylen, unsigned char *h1,
-			unsigned char *h2, unsigned char h3, ...)
+			unsigned char *h2, int h3, ...)
 {
 	unsigned char paramdigest[SHA1_DIGEST_SIZE];
 	struct sdesc *sdesc;
@@ -139,7 +139,7 @@ static int TSS_authhmac(unsigned char *digest, const unsigned char *key,
 		return PTR_ERR(sdesc);
 	}
 
-	c = h3;
+	c = (unsigned char) h3;
 	ret = crypto_shash_init(&sdesc->shash);
 	if (ret < 0)
 		goto out;

is the simplest (correct) solution.

@nathanchance
Copy link
Member

Seems reasonable to me, I'll test it later tonight with the two outstanding issues of this type and prepare a couple of patches, thanks for looking into this.

@nickdesaulniers
Copy link
Member Author

nickdesaulniers commented Oct 9, 2018

@nickdesaulniers
Copy link
Member Author

nickdesaulniers commented Oct 11, 2018

@nickdesaulniers nickdesaulniers self-assigned this Oct 11, 2018
@nickdesaulniers nickdesaulniers added the [PATCH] Submitted A patch has been submitted for review label Oct 11, 2018
fengguang pushed a commit to 0day-ci/linux that referenced this issue Oct 12, 2018
by swapping h2 and h3.

security/keys/trusted.c:146:17: warning: passing an object that
undergoes default
      argument promotion to 'va_start' has undefined behavior [-Wvarargs]
        va_start(argp, h3);
                       ^
security/keys/trusted.c:126:37: note: parameter of type 'unsigned
char' is declared here
unsigned char *h2, unsigned char h3, ...)
                               ^

Specifically, it seems that both the C90 (4.8.1.1) and C11 (7.16.1.4)
standards explicitly call this out as undefined behavior:

The parameter parmN is the identifier of the rightmost parameter in
the variable parameter list in the function definition (the one just
before the ...). If the parameter parmN is declared with ... or with a
type that is not compatible with the type that results after
application of the default argument promotions, the behavior is
undefined.

Link: ClangBuiltLinux#41
Suggested-by: James Bottomley <jejb@linux.vnet.ibm.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
@nathanchance nathanchance added [BUG] linux A bug that should be fixed in the mainline kernel. and removed [BUG] Untriaged Something isn't working labels Oct 13, 2018
@nickdesaulniers
Copy link
Member Author

nickdesaulniers commented Oct 22, 2018

https://lore.kernel.org/r/20181022234357.82217-1-ndesaulniers@google.com/: [PATCH v2] [PATCH] KEYS: trusted: fix -Wvarags warning

nathanchance pushed a commit that referenced this issue Nov 7, 2018
Fix double-free that happens when thermal zone setup fails, see KASAN log
below.

==================================================================
BUG: KASAN: double-free or invalid-free in __hwmon_device_register+0x5dc/0xa7c

CPU: 0 PID: 132 Comm: kworker/0:2 Tainted: G    B             4.19.0-rc8-next-20181016-00042-gb52cd80401e9-dirty #41
Hardware name: NVIDIA Tegra SoC (Flattened Device Tree)
Workqueue: events deferred_probe_work_func
Backtrace:
[<c0110540>] (dump_backtrace) from [<c0110944>] (show_stack+0x20/0x24)
[<c0110924>] (show_stack) from [<c105cb08>] (dump_stack+0x9c/0xb0)
[<c105ca6c>] (dump_stack) from [<c02fdaec>] (print_address_description+0x68/0x250)
[<c02fda84>] (print_address_description) from [<c02fd4ac>] (kasan_report_invalid_free+0x68/0x88)
[<c02fd444>] (kasan_report_invalid_free) from [<c02fc85c>] (__kasan_slab_free+0x1f4/0x200)
[<c02fc668>] (__kasan_slab_free) from [<c02fd0c0>] (kasan_slab_free+0x14/0x18)
[<c02fd0ac>] (kasan_slab_free) from [<c02f9c6c>] (kfree+0x90/0x294)
[<c02f9bdc>] (kfree) from [<c0b41bbc>] (__hwmon_device_register+0x5dc/0xa7c)
[<c0b415e0>] (__hwmon_device_register) from [<c0b421e8>] (hwmon_device_register_with_info+0xa0/0xa8)
[<c0b42148>] (hwmon_device_register_with_info) from [<c0b42324>] (devm_hwmon_device_register_with_info+0x74/0xb4)
[<c0b422b0>] (devm_hwmon_device_register_with_info) from [<c0b4481c>] (lm90_probe+0x414/0x578)
[<c0b44408>] (lm90_probe) from [<c0aeeff4>] (i2c_device_probe+0x35c/0x384)
[<c0aeec98>] (i2c_device_probe) from [<c08776cc>] (really_probe+0x290/0x3e4)
[<c087743c>] (really_probe) from [<c0877a2c>] (driver_probe_device+0x80/0x1c4)
[<c08779ac>] (driver_probe_device) from [<c0877da8>] (__device_attach_driver+0x104/0x11c)
[<c0877ca4>] (__device_attach_driver) from [<c0874dd8>] (bus_for_each_drv+0xa4/0xc8)
[<c0874d34>] (bus_for_each_drv) from [<c08773b0>] (__device_attach+0xf0/0x15c)
[<c08772c0>] (__device_attach) from [<c0877e24>] (device_initial_probe+0x1c/0x20)
[<c0877e08>] (device_initial_probe) from [<c08762f4>] (bus_probe_device+0xdc/0xec)
[<c0876218>] (bus_probe_device) from [<c0876a08>] (deferred_probe_work_func+0xa8/0xd4)
[<c0876960>] (deferred_probe_work_func) from [<c01527c4>] (process_one_work+0x3dc/0x96c)
[<c01523e8>] (process_one_work) from [<c01541e0>] (worker_thread+0x4ec/0x8bc)
[<c0153cf4>] (worker_thread) from [<c015b238>] (kthread+0x230/0x240)
[<c015b008>] (kthread) from [<c01010bc>] (ret_from_fork+0x14/0x38)
Exception stack(0xcf743fb0 to 0xcf743ff8)
3fa0:                                     00000000 00000000 00000000 00000000
3fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
3fe0: 00000000 00000000 00000000 00000000 00000013 00000000

Allocated by task 132:
 kasan_kmalloc.part.1+0x58/0xf4
 kasan_kmalloc+0x90/0xa4
 kmem_cache_alloc_trace+0x90/0x2a0
 __hwmon_device_register+0xbc/0xa7c
 hwmon_device_register_with_info+0xa0/0xa8
 devm_hwmon_device_register_with_info+0x74/0xb4
 lm90_probe+0x414/0x578
 i2c_device_probe+0x35c/0x384
 really_probe+0x290/0x3e4
 driver_probe_device+0x80/0x1c4
 __device_attach_driver+0x104/0x11c
 bus_for_each_drv+0xa4/0xc8
 __device_attach+0xf0/0x15c
 device_initial_probe+0x1c/0x20
 bus_probe_device+0xdc/0xec
 deferred_probe_work_func+0xa8/0xd4
 process_one_work+0x3dc/0x96c
 worker_thread+0x4ec/0x8bc
 kthread+0x230/0x240
 ret_from_fork+0x14/0x38
   (null)

Freed by task 132:
 __kasan_slab_free+0x12c/0x200
 kasan_slab_free+0x14/0x18
 kfree+0x90/0x294
 hwmon_dev_release+0x1c/0x20
 device_release+0x4c/0xe8
 kobject_put+0xac/0x11c
 device_unregister+0x2c/0x30
 __hwmon_device_register+0xa58/0xa7c
 hwmon_device_register_with_info+0xa0/0xa8
 devm_hwmon_device_register_with_info+0x74/0xb4
 lm90_probe+0x414/0x578
 i2c_device_probe+0x35c/0x384
 really_probe+0x290/0x3e4
 driver_probe_device+0x80/0x1c4
 __device_attach_driver+0x104/0x11c
 bus_for_each_drv+0xa4/0xc8
 __device_attach+0xf0/0x15c
 device_initial_probe+0x1c/0x20
 bus_probe_device+0xdc/0xec
 deferred_probe_work_func+0xa8/0xd4
 process_one_work+0x3dc/0x96c
 worker_thread+0x4ec/0x8bc
 kthread+0x230/0x240
 ret_from_fork+0x14/0x38
   (null)

Cc: <stable@vger.kernel.org> # v4.15+
Fixes: 47c332d ("hwmon: Deal with errors from the thermal subsystem")
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
nathanchance pushed a commit that referenced this issue Nov 9, 2018
Increase kasan instrumented kernel stack size from 32k to 64k. Other
architectures seems to get away with just doubling kernel stack size under
kasan, but on s390 this appears to be not enough due to bigger frame size.
The particular pain point is kasan inlined checks (CONFIG_KASAN_INLINE
vs CONFIG_KASAN_OUTLINE). With inlined checks one particular case hitting
stack overflow is fs sync on xfs filesystem:

 #0 [9a0681e8]  704 bytes  check_usage at 34b1fc
 #1 [9a0684a8]  432 bytes  check_usage at 34c710
 #2 [9a068658]  1048 bytes  validate_chain at 35044a
 #3 [9a068a70]  312 bytes  __lock_acquire at 3559fe
 #4 [9a068ba8]  440 bytes  lock_acquire at 3576ee
 #5 [9a068d60]  104 bytes  _raw_spin_lock at 21b44e0
 #6 [9a068dc8]  1992 bytes  enqueue_entity at 2dbf72
 #7 [9a069590]  1496 bytes  enqueue_task_fair at 2df5f0
 #8 [9a069b68]  64 bytes  ttwu_do_activate at 28f438
 #9 [9a069ba8]  552 bytes  try_to_wake_up at 298c4c
 #10 [9a069dd0]  168 bytes  wake_up_worker at 23f97c
 #11 [9a069e78]  200 bytes  insert_work at 23fc2e
 #12 [9a069f40]  648 bytes  __queue_work at 2487c0
 #13 [9a06a1c8]  200 bytes  __queue_delayed_work at 24db28
 #14 [9a06a290]  248 bytes  mod_delayed_work_on at 24de84
 #15 [9a06a388]  24 bytes  kblockd_mod_delayed_work_on at 153e2a0
 #16 [9a06a3a0]  288 bytes  __blk_mq_delay_run_hw_queue at 158168c
 #17 [9a06a4c0]  192 bytes  blk_mq_run_hw_queue at 1581a3c
 #18 [9a06a580]  184 bytes  blk_mq_sched_insert_requests at 15a2192
 #19 [9a06a638]  1024 bytes  blk_mq_flush_plug_list at 1590f3a
 #20 [9a06aa38]  704 bytes  blk_flush_plug_list at 1555028
 #21 [9a06acf8]  320 bytes  schedule at 219e476
 #22 [9a06ae38]  760 bytes  schedule_timeout at 21b0aac
 #23 [9a06b130]  408 bytes  wait_for_common at 21a1706
 #24 [9a06b2c8]  360 bytes  xfs_buf_iowait at fa1540
 #25 [9a06b430]  256 bytes  __xfs_buf_submit at fadae6
 #26 [9a06b530]  264 bytes  xfs_buf_read_map at fae3f6
 #27 [9a06b638]  656 bytes  xfs_trans_read_buf_map at 10ac9a8
 #28 [9a06b8c8]  304 bytes  xfs_btree_kill_root at e72426
 #29 [9a06b9f8]  288 bytes  xfs_btree_lookup_get_block at e7bc5e
 #30 [9a06bb18]  624 bytes  xfs_btree_lookup at e7e1a6
 #31 [9a06bd88]  2664 bytes  xfs_alloc_ag_vextent_near at dfa070
 #32 [9a06c7f0]  144 bytes  xfs_alloc_ag_vextent at dff3ca
 #33 [9a06c880]  1128 bytes  xfs_alloc_vextent at e05fce
 #34 [9a06cce8]  584 bytes  xfs_bmap_btalloc at e58342
 #35 [9a06cf30]  1336 bytes  xfs_bmapi_write at e618de
 #36 [9a06d468]  776 bytes  xfs_iomap_write_allocate at ff678e
 #37 [9a06d770]  720 bytes  xfs_map_blocks at f82af8
 #38 [9a06da40]  928 bytes  xfs_writepage_map at f83cd6
 #39 [9a06dde0]  320 bytes  xfs_do_writepage at f85872
 #40 [9a06df20]  1320 bytes  write_cache_pages at 73dfe8
 #41 [9a06e448]  208 bytes  xfs_vm_writepages at f7f892
 #42 [9a06e518]  88 bytes  do_writepages at 73fe6a
 #43 [9a06e570]  872 bytes  __writeback_single_inode at a20cb6
 #44 [9a06e8d8]  664 bytes  writeback_sb_inodes at a23be2
 #45 [9a06eb70]  296 bytes  __writeback_inodes_wb at a242e0
 #46 [9a06ec98]  928 bytes  wb_writeback at a2500e
 #47 [9a06f038]  848 bytes  wb_do_writeback at a260ae
 #48 [9a06f388]  536 bytes  wb_workfn at a28228
 #49 [9a06f5a0]  1088 bytes  process_one_work at 24a234
 #50 [9a06f9e0]  1120 bytes  worker_thread at 24ba26
 #51 [9a06fe40]  104 bytes  kthread at 26545a
 #52 [9a06fea8]             kernel_thread_starter at 21b6b62

To be able to increase the stack size to 64k reuse LLILL instruction
in __switch_to function to load 64k - STACK_FRAME_OVERHEAD - __PT_SIZE
(65192) value as unsigned.

Reported-by: Benjamin Block <bblock@linux.ibm.com>
Reviewed-by: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Vasily Gorbik <gor@linux.ibm.com>
Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
nathanchance pushed a commit that referenced this issue Dec 20, 2018
Add napi_disable routine in gro_cells_destroy since starting from
commit c42858e ("gro_cells: remove spinlock protecting receive
queues") gro_cell_poll and gro_cells_destroy can run concurrently on
napi_skbs list producing a kernel Oops if the tunnel interface is
removed while gro_cell_poll is running. The following Oops has been
triggered removing a vxlan device while the interface is receiving
traffic

[ 5628.948853] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
[ 5628.949981] PGD 0 P4D 0
[ 5628.950308] Oops: 0002 [#1] SMP PTI
[ 5628.950748] CPU: 0 PID: 9 Comm: ksoftirqd/0 Not tainted 4.20.0-rc6+ #41
[ 5628.952940] RIP: 0010:gro_cell_poll+0x49/0x80
[ 5628.955615] RSP: 0018:ffffc9000004fdd8 EFLAGS: 00010202
[ 5628.956250] RAX: 0000000000000000 RBX: ffffe8ffffc08150 RCX: 0000000000000000
[ 5628.957102] RDX: 0000000000000000 RSI: ffff88802356bf00 RDI: ffffe8ffffc08150
[ 5628.957940] RBP: 0000000000000026 R08: 0000000000000000 R09: 0000000000000000
[ 5628.958803] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000040
[ 5628.959661] R13: ffffe8ffffc08100 R14: 0000000000000000 R15: 0000000000000040
[ 5628.960682] FS:  0000000000000000(0000) GS:ffff88803ea00000(0000) knlGS:0000000000000000
[ 5628.961616] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 5628.962359] CR2: 0000000000000008 CR3: 000000000221c000 CR4: 00000000000006b0
[ 5628.963188] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 5628.964034] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 5628.964871] Call Trace:
[ 5628.965179]  net_rx_action+0xf0/0x380
[ 5628.965637]  __do_softirq+0xc7/0x431
[ 5628.966510]  run_ksoftirqd+0x24/0x30
[ 5628.966957]  smpboot_thread_fn+0xc5/0x160
[ 5628.967436]  kthread+0x113/0x130
[ 5628.968283]  ret_from_fork+0x3a/0x50
[ 5628.968721] Modules linked in:
[ 5628.969099] CR2: 0000000000000008
[ 5628.969510] ---[ end trace 9d9dedc7181661fe ]---
[ 5628.970073] RIP: 0010:gro_cell_poll+0x49/0x80
[ 5628.972965] RSP: 0018:ffffc9000004fdd8 EFLAGS: 00010202
[ 5628.973611] RAX: 0000000000000000 RBX: ffffe8ffffc08150 RCX: 0000000000000000
[ 5628.974504] RDX: 0000000000000000 RSI: ffff88802356bf00 RDI: ffffe8ffffc08150
[ 5628.975462] RBP: 0000000000000026 R08: 0000000000000000 R09: 0000000000000000
[ 5628.976413] R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000040
[ 5628.977375] R13: ffffe8ffffc08100 R14: 0000000000000000 R15: 0000000000000040
[ 5628.978296] FS:  0000000000000000(0000) GS:ffff88803ea00000(0000) knlGS:0000000000000000
[ 5628.979327] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 5628.980044] CR2: 0000000000000008 CR3: 000000000221c000 CR4: 00000000000006b0
[ 5628.980929] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 5628.981736] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 5628.982409] Kernel panic - not syncing: Fatal exception in interrupt
[ 5628.983307] Kernel Offset: disabled

Fixes: c42858e ("gro_cells: remove spinlock protecting receive queues")
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Acked-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
@nickdesaulniers
Copy link
Member Author

bumped the maintainers again.

@nathanchance
Copy link
Member

@nathanchance nathanchance added [PATCH] Accepted A submitted patch has been accepted upstream and removed [PATCH] Submitted A patch has been submitted for review labels Feb 14, 2019
@tpimh tpimh removed the low priority This bug is not critical and not a priority label Mar 10, 2019
raphielscape pushed a commit to raphielscape/linux-scape-workstation that referenced this issue Apr 5, 2019
Fixes the warning reported by Clang:
security/keys/trusted.c:146:17: warning: passing an object that
undergoes default
      argument promotion to 'va_start' has undefined behavior [-Wvarargs]
        va_start(argp, h3);
                       ^
security/keys/trusted.c:126:37: note: parameter of type 'unsigned
char' is declared here
unsigned char *h2, unsigned char h3, ...)
                               ^
Specifically, it seems that both the C90 (4.8.1.1) and C11 (7.16.1.4)
standards explicitly call this out as undefined behavior:

The parameter parmN is the identifier of the rightmost parameter in
the variable parameter list in the function definition (the one just
before the ...). If the parameter parmN is declared with ... or with a
type that is not compatible with the type that results after
application of the default argument promotions, the behavior is
undefined.

Link: ClangBuiltLinux/linux#41
Link: https://www.eskimo.com/~scs/cclass/int/sx11c.html
Suggested-by: David Laight <David.Laight@aculab.com>
Suggested-by: Denis Kenzior <denkenz@gmail.com>
Suggested-by: James Bottomley <jejb@linux.vnet.ibm.com>
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Albert I <krascgq@outlook.co.id>
Signed-off-by: Raphiel Rollerscaperers <raphielscape@outlook.com>
@nathanchance
Copy link
Member

@nathanchance nathanchance removed the [PATCH] Accepted A submitted patch has been accepted upstream label Apr 9, 2019
@nathanchance nathanchance added the [FIXED][LINUX] 5.1 This bug was fixed in Linux 5.1 label Apr 9, 2019
@nickdesaulniers
Copy link
Member Author

Merged into mainline

six months later... 💀 ☠️ 👻

avagin pushed a commit to avagin/linux that referenced this issue Apr 11, 2019
Fixes the warning reported by Clang:
security/keys/trusted.c:146:17: warning: passing an object that
undergoes default
      argument promotion to 'va_start' has undefined behavior [-Wvarargs]
        va_start(argp, h3);
                       ^
security/keys/trusted.c:126:37: note: parameter of type 'unsigned
char' is declared here
unsigned char *h2, unsigned char h3, ...)
                               ^
Specifically, it seems that both the C90 (4.8.1.1) and C11 (7.16.1.4)
standards explicitly call this out as undefined behavior:

The parameter parmN is the identifier of the rightmost parameter in
the variable parameter list in the function definition (the one just
before the ...). If the parameter parmN is declared with ... or with a
type that is not compatible with the type that results after
application of the default argument promotions, the behavior is
undefined.

Link: ClangBuiltLinux/linux#41
Link: https://www.eskimo.com/~scs/cclass/int/sx11c.html
Suggested-by: David Laight <David.Laight@aculab.com>
Suggested-by: Denis Kenzior <denkenz@gmail.com>
Suggested-by: James Bottomley <jejb@linux.vnet.ibm.com>
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
rgushchin pushed a commit to rgushchin/linux that referenced this issue Apr 11, 2019
GIT 869e330

commit 8e44e96
Author: Michael Chan <michael.chan@broadcom.com>
Date:   Mon Apr 8 17:39:55 2019 -0400

    bnxt_en: Reset device on RX buffer errors.
    
    If the RX completion indicates RX buffers errors, the RX ring will be
    disabled by firmware and no packets will be received on that ring from
    that point on.  Recover by resetting the device.
    
    Fixes: c0c050c ("bnxt_en: New Broadcom ethernet driver.")
    Signed-off-by: Michael Chan <michael.chan@broadcom.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit a1b0e4e
Author: Michael Chan <michael.chan@broadcom.com>
Date:   Mon Apr 8 17:39:54 2019 -0400

    bnxt_en: Improve RX consumer index validity check.
    
    There is logic to check that the RX/TPA consumer index is the expected
    index to work around a hardware problem.  However, the potentially bad
    consumer index is first used to index into an array to reference an entry.
    This can potentially crash if the bad consumer index is beyond legal
    range.  Improve the logic to use the consumer index for dereferencing
    after the validity check and log an error message.
    
    Fixes: fa7e281 ("bnxt_en: Add workaround to detect bad opaque in rx completion (part 2)")
    Signed-off-by: Michael Chan <michael.chan@broadcom.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit a625204
Author: Paul Thomas <pthomas8589@gmail.com>
Date:   Mon Apr 8 15:37:54 2019 -0400

    net: macb driver, check for SKBTX_HW_TSTAMP
    
    Make sure SKBTX_HW_TSTAMP (i.e. SOF_TIMESTAMPING_TX_HARDWARE) has been
    enabled for this skb. It does fix the issue where normal socks that
    aren't expecting a timestamp will not wake up on select, but when a
    user does want a SOF_TIMESTAMPING_TX_HARDWARE it does work.
    
    Signed-off-by: Paul Thomas <pthomas8589@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit d63da85
Author: Michael Zhivich <mzhivich@akamai.com>
Date:   Mon Apr 8 10:48:47 2019 -0400

    qlogic: qlcnic: fix use of SPEED_UNKNOWN ethtool constant
    
    qlcnic driver uses u16 to store SPEED_UKNOWN ethtool constant,
    which is defined as -1, resulting in value truncation and
    thus incorrect test results against SPEED_UNKNOWN.
    
    For example, the following test will print "False":
    
        u16 speed = SPEED_UNKNOWN;
    
        if (speed == SPEED_UNKNOWN)
            printf("True");
        else
            printf("False");
    
    Change storage of speed to use u32 to avoid this issue.
    
    Signed-off-by: Michael Zhivich <mzhivich@akamai.com>
    Reviewed-by: Andrew Lunn <andrew@lunn.ch>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit caf2c52
Author: Michael Zhivich <mzhivich@akamai.com>
Date:   Mon Apr 8 10:48:46 2019 -0400

    broadcom: tg3: fix use of SPEED_UNKNOWN ethtool constant
    
    tg3 driver uses u16 to store SPEED_UKNOWN ethtool constant,
    which is defined as -1, resulting in value truncation and
    thus incorrect test results against SPEED_UNKNOWN.
    
    For example, the following test will print "False":
    
            u16 speed = SPEED_UNKNOWN;
    
            if (speed == SPEED_UNKNOWN)
                printf("True");
            else
                printf("False");
    
    Change storage of speed to use u32 to avoid this issue.
    
    Signed-off-by: Michael Zhivich <mzhivich@akamai.com>
    Reviewed-by: Andrew Lunn <andrew@lunn.ch>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit afe6424
Author: Michael Zhivich <mzhivich@akamai.com>
Date:   Mon Apr 8 10:48:45 2019 -0400

    ethtool: avoid signed-unsigned comparison in ethtool_validate_speed()
    
    When building C++ userspace code that includes ethtool.h
    with "-Werror -Wall", g++ complains about signed-unsigned comparison in
    ethtool_validate_speed() due to definition of SPEED_UNKNOWN as -1.
    
    Explicitly cast SPEED_UNKNOWN to __u32 to match type of
    ethtool_validate_speed() argument.
    
    Signed-off-by: Michael Zhivich <mzhivich@akamai.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit 2a3caba
Author: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date:   Sat Apr 6 17:16:53 2019 +0200

    net: ip6_gre: fix possible use-after-free in ip6erspan_rcv
    
    erspan_v6 tunnels run __iptunnel_pull_header on received skbs to remove
    erspan header. This can determine a possible use-after-free accessing
    pkt_md pointer in ip6erspan_rcv since the packet will be 'uncloned'
    running pskb_expand_head if it is a cloned gso skb (e.g if the packet has
    been sent though a veth device). Fix it resetting pkt_md pointer after
    __iptunnel_pull_header
    
    Fixes: 1d7e2ed ("net: erspan: refactor existing erspan code")
    Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit 492b67e
Author: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Date:   Sat Apr 6 17:16:52 2019 +0200

    net: ip_gre: fix possible use-after-free in erspan_rcv
    
    erspan tunnels run __iptunnel_pull_header on received skbs to remove
    gre and erspan headers. This can determine a possible use-after-free
    accessing pkt_md pointer in erspan_rcv since the packet will be 'uncloned'
    running pskb_expand_head if it is a cloned gso skb (e.g if the packet has
    been sent though a veth device). Fix it resetting pkt_md pointer after
    __iptunnel_pull_header
    
    Fixes: 1d7e2ed ("net: erspan: refactor existing erspan code")
    Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit 6da7058
Author: Tadeusz Struk <tadeusz.struk@intel.com>
Date:   Tue Feb 12 15:42:05 2019 -0800

    selftests/tpm2: Open tpm dev in unbuffered mode
    
    In order to have control over how many bytes are read or written
    the device needs to be opened in unbuffered mode.
    
    Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
    Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: James Morris <james.morris@microsoft.com>

commit f1a0ba6
Author: Tadeusz Struk <tadeusz.struk@intel.com>
Date:   Tue Feb 12 15:42:10 2019 -0800

    selftests/tpm2: Extend tests to cover partial reads
    
    Three new tests added:
    1. Send get random cmd, read header in 1st read, read the rest in second
       read - expect success
    2. Send get random cmd, read only part of the response, send another
       get random command, read the response - expect success
    3. Send get random cmd followed by another get random cmd, without
       reading the first response - expect the second cmd to fail with -EBUSY
    
    Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
    Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Tested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: James Morris <james.morris@microsoft.com>

commit be24b37
Author: ndesaulniers@google.com <ndesaulniers@google.com>
Date:   Mon Oct 22 16:43:57 2018 -0700

    KEYS: trusted: fix -Wvarags warning
    
    Fixes the warning reported by Clang:
    security/keys/trusted.c:146:17: warning: passing an object that
    undergoes default
          argument promotion to 'va_start' has undefined behavior [-Wvarargs]
            va_start(argp, h3);
                           ^
    security/keys/trusted.c:126:37: note: parameter of type 'unsigned
    char' is declared here
    unsigned char *h2, unsigned char h3, ...)
                                   ^
    Specifically, it seems that both the C90 (4.8.1.1) and C11 (7.16.1.4)
    standards explicitly call this out as undefined behavior:
    
    The parameter parmN is the identifier of the rightmost parameter in
    the variable parameter list in the function definition (the one just
    before the ...). If the parameter parmN is declared with ... or with a
    type that is not compatible with the type that results after
    application of the default argument promotions, the behavior is
    undefined.
    
    Link: ClangBuiltLinux#41
    Link: https://www.eskimo.com/~scs/cclass/int/sx11c.html
    Suggested-by: David Laight <David.Laight@aculab.com>
    Suggested-by: Denis Kenzior <denkenz@gmail.com>
    Suggested-by: James Bottomley <jejb@linux.vnet.ibm.com>
    Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
    Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
    Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
    Tested-by: Nathan Chancellor <natechancellor@gmail.com>
    Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: James Morris <james.morris@microsoft.com>

commit b9d0a85
Author: Yue Haibing <yuehaibing@huawei.com>
Date:   Wed Feb 20 16:25:38 2019 +0800

    tpm: Fix the type of the return value in calc_tpm2_event_size()
    
    calc_tpm2_event_size() has an invalid signature because
    it returns a 'size_t' where as its signature says that
    it returns 'int'.
    
    Cc: <stable@vger.kernel.org>
    Fixes: 4d23cc3 ("tpm: add securityfs support for TPM 2.0 firmware event log")
    Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: Yue Haibing <yuehaibing@huawei.com>
    Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: James Morris <james.morris@microsoft.com>

commit c787192
Author: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date:   Mon Mar 25 16:43:10 2019 +0200

    KEYS: trusted: allow trusted.ko to initialize w/o a TPM
    
    Allow trusted.ko to initialize w/o a TPM. This commit also adds checks
    to the exported functions to fail when a TPM is not available.
    
    Fixes: 2407304 ("KEYS: trusted: explicitly use tpm_chip structure...")
    Cc: James Morris <jmorris@namei.org>
    Reported-by: Dan Williams <dan.j.williams@intel.com>
    Tested-by: Dan Williams <dan.j.williams@intel.com>
    Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: James Morris <james.morris@microsoft.com>

commit 7110629
Author: Tadeusz Struk <tadeusz.struk@intel.com>
Date:   Wed Mar 27 11:32:38 2019 -0700

    tpm: fix an invalid condition in tpm_common_poll
    
    The poll condition should only check response_length,
    because reads should only be issued if there is data to read.
    The response_read flag only prevents double writes.
    The problem was that the write set the response_read to false,
    enqued a tpm job, and returned. Then application called poll
    which checked the response_read flag and returned EPOLLIN.
    Then the application called read, but got nothing.
    After all that the async_work kicked in.
    Added also mutex_lock around the poll check to prevent
    other possible race conditions.
    
    Fixes: 9488585 ("tpm: add support for partial reads")
    Reported-by: Mantas Mikulėnas <grawity@gmail.com>
    Tested-by: Mantas Mikulėnas <grawity@gmail.com>
    Signed-off-by: Tadeusz Struk <tadeusz.struk@intel.com>
    Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Signed-off-by: James Morris <james.morris@microsoft.com>

commit e891db1
Author: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Date:   Fri Mar 22 12:51:20 2019 +0200

    tpm: turn on TPM on suspend for TPM 1.x
    
    tpm_chip_start/stop() should be also called for TPM 1.x devices on
    suspend. Add that functionality back. Do not lock the chip because
    it is unnecessary as there are no multiple threads using it when
    doing the suspend.
    
    Fixes: a3fbfae ("tpm: take TPM chip power gating out of tpm_transmit()")
    Reported-by: Paul Zimmerman <pauldzim@gmail.com>
    Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
    Tested-by: Domenico Andreoli <domenico.andreoli@linux.com>
    Signed-off-by: James Morris <james.morris@microsoft.com>

commit b75bb8a
Author: Heiner Kallweit <hkallweit1@gmail.com>
Date:   Fri Apr 5 20:46:46 2019 +0200

    r8169: disable ASPM again
    
    There's a significant number of reports that re-enabling ASPM causes
    different issues, ranging from decreased performance to system not
    booting at all. This affects only a minority of users, but the number
    of affected users is big enough that we better switch off ASPM again.
    
    This will hurt notebook users who are not affected by the issues, they
    may see decreased battery runtime w/o ASPM. With the PCI core folks is
    being discussed to add generic sysfs attributes to control ASPM.
    Once this is in place brave enough users can re-enable ASPM on their
    system.
    
    Fixes: a99790b ("r8169: Reinstate ASPM Support")
    Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit b1a6e8f
Author: Stefan Schmidt <stefan@datenfreihafen.org>
Date:   Mon Apr 8 18:08:04 2019 +0200

    MAINTAINERS: ieee802154: update documentation file pattern
    
    When moving the documentation for the ieee802154 subsystem from
    plain text to rst the file pattern in the MAINTAINERS file got wrong.
    Updating it here to fix scripts using this file.
    
    Reported-by: Joe Perches <joe@perches.com>
    Signed-off-by: Stefan Schmidt <stefan@datenfreihafen.org>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit 5055376
Author: Miaohe Lin <linmiaohe@huawei.com>
Date:   Mon Apr 8 10:04:20 2019 +0800

    net: vrf: Fix ping failed when vrf mtu is set to 0
    
    When the mtu of a vrf device is set to 0, it would cause ping
    failed. So I think we should limit vrf mtu in a reasonable range
    to solve this problem. I set dev->min_mtu to IPV6_MIN_MTU, so it
    will works for both ipv4 and ipv6. And if dev->max_mtu still be 0
    can be confusing, so I set dev->max_mtu to ETH_MAX_MTU.
    
    Here is the reproduce step:
    
    1.Config vrf interface and set mtu to 0:
    3: enp4s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel
    master vrf1 state UP mode DEFAULT group default qlen 1000
        link/ether 52:54:00:9e:dd:c1 brd ff:ff:ff:ff:ff:ff
    
    2.Ping peer:
    3: enp4s0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc fq_codel
    master vrf1 state UP group default qlen 1000
        link/ether 52:54:00:9e:dd:c1 brd ff:ff:ff:ff:ff:ff
        inet 10.0.0.1/16 scope global enp4s0
           valid_lft forever preferred_lft forever
    connect: Network is unreachable
    
    3.Set mtu to default value, ping works:
    PING 10.0.0.2 (10.0.0.2) 56(84) bytes of data.
    64 bytes from 10.0.0.2: icmp_seq=1 ttl=64 time=1.88 ms
    
    Fixes: ad49bc6 ("net: vrf: remove MTU limits for vrf device")
    Signed-off-by: Miaohe Lin <linmiaohe@huawei.com>
    Reviewed-by: David Ahern <dsahern@gmail.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit fcf8891
Author: Qian Cai <cai@lca.pw>
Date:   Sat Apr 6 18:59:01 2019 -0400

    slab: fix a crash by reading /proc/slab_allocators
    
    The commit 510ded3 ("slab: implement slab_root_caches list")
    changes the name of the list node within "struct kmem_cache" from "list"
    to "root_caches_node", but leaks_show() still use the "list" which
    causes a crash when reading /proc/slab_allocators.
    
    You need to have CONFIG_SLAB=y and CONFIG_MEMCG=y to see the problem,
    because without MEMCG all slab caches are root caches, and the "list"
    node happens to be the right one.
    
    Fixes: 510ded3 ("slab: implement slab_root_caches list")
    Signed-off-by: Qian Cai <cai@lca.pw>
    Reviewed-by: Tobin C. Harding <tobin@kernel.org>
    Cc: Tejun Heo <tj@kernel.org>
    Cc: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

commit b959ecf
Author: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date:   Fri Apr 5 14:20:24 2019 +0200

    selftests: add a tc matchall test case
    
    This is a follow up of the commit 0db6f8b ("net/sched: fix ->get
    helper of the matchall cls").
    
    To test it:
    $ cd tools/testing/selftests/tc-testing
    $ ln -s ../plugin-lib/nsPlugin.py plugins/20-nsPlugin.py
    $ ./tdc.py -n -e 2638
    
    Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit 6491d69
Author: Dan Carpenter <dan.carpenter@oracle.com>
Date:   Wed Apr 3 10:13:51 2019 +0300

    nfc: nci: Potential off by one in ->pipes[] array
    
    This is similar to commit e285d5b ("NFC: Fix the number of pipes")
    where we changed NFC_HCI_MAX_PIPES from 127 to 128.
    
    As the comment next to the define explains, the pipe identifier is 7
    bits long.  The highest possible pipe is 127, but the number of possible
    pipes is 128.  As the code is now, then there is potential for an
    out of bounds array access:
    
        net/nfc/nci/hci.c:297 nci_hci_cmd_received() warn: array off by one?
        'ndev->hci_dev->pipes[pipe]' '0-127 == 127'
    
    Fixes: 11f54f2 ("NFC: nci: Add HCI over NCI protocol support")
    Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit d7ee81a
Author: Dan Carpenter <dan.carpenter@oracle.com>
Date:   Wed Apr 3 10:12:48 2019 +0300

    NFC: nci: Add some bounds checking in nci_hci_cmd_received()
    
    This is similar to commit 674d9de ("NFC: Fix possible memory
    corruption when handling SHDLC I-Frame commands").
    
    I'm not totally sure, but I think that commit description may have
    overstated the danger.  I was under the impression that this data came
    from the firmware?  If you can't trust your networking firmware, then
    you're already in trouble.
    
    Anyway, these days we add bounds checking where ever we can and we call
    it kernel hardening.  Better safe than sorry.
    
    Fixes: 11f54f2 ("NFC: nci: Add HCI over NCI protocol support")
    Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

commit ecae26f
Author: Max Filippov <jcmvbkbc@gmail.com>
Date:   Thu Apr 4 18:42:05 2019 -0700

    xtensa: fix format string warning in init_pmd
    
    Use %lu instead of %zu to fix the following warning introduced with
    recent memblock refactoring:
      xtensa/mm/mmu.c:36:9: warning: format '%zu' expects argument of type
      'size_t', but argument 3 has type 'long unsigned int
    
    Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>

commit ada770b
Author: Max Filippov <jcmvbkbc@gmail.com>
Date:   Thu Apr 4 11:08:40 2019 -0700

    xtensa: fix return_address
    
    return_address returns the address that is one level higher in the call
    stack than requested in its argument, because level 0 corresponds to its
    caller's return address. Use requested level as the number of stack
    frames to skip.
    
    This fixes the address reported by might_sleep and friends.
    
    Cc: stable@vger.kernel.org
    Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>

commit 2663147
Author: Max Filippov <jcmvbkbc@gmail.com>
Date:   Wed Apr 3 20:22:42 2019 -0700

    xtensa: fix initialization of pt_regs::syscall in start_thread
    
    New pt_regs should indicate that there's no syscall, not that there's
    syscall #0. While at it wrap macro body in do/while and parenthesize
    macro arguments.
    
    Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>

commit 2201f31
Author: Max Filippov <jcmvbkbc@gmail.com>
Date:   Wed Apr 3 20:19:25 2019 -0700

    xtensa: use actual syscall number in do_syscall_trace_leave
    
    Syscall may alter pt_regs structure passed to it, resulting in a
    mismatch between syscall entry end syscall exit entries in the ftrace.
    Temporary restore syscall field of the pt_regs for the duration of
    do_syscall_trace_leave.
    
    Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>

commit b4e9e93
Author: Iuliana Prodan <iuliana.prodan@nxp.com>
Date:   Fri Mar 22 14:12:30 2019 +0200

    crypto: caam - fix copy of next buffer for xcbc and cmac
    
    Fix a side effect of adding xcbc support, when the next_buffer is not
    copied.
    The issue occurs, when there is stored from previous state a blocksize
    buffer and received, a less than blocksize, from user. In this case, the
    nents for req->src is 0, and the next_buffer is not copied.
    An example is:
    {
            .tap    = { 17, 15, 8 },
            .psize  = 40,
            .np     = 3,
            .ksize  = 16,
    }
    
    Fixes: 12b8567 ("crypto: caam - add support for xcbc(aes)")
    Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>
    Reviewed-by: Horia Geantă <horia.geanta@nxp.com>
    Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Whissi pushed a commit to Whissi/linux-stable that referenced this issue May 10, 2019
[ Upstream commit be24b37 ]

Fixes the warning reported by Clang:
security/keys/trusted.c:146:17: warning: passing an object that
undergoes default
      argument promotion to 'va_start' has undefined behavior [-Wvarargs]
        va_start(argp, h3);
                       ^
security/keys/trusted.c:126:37: note: parameter of type 'unsigned
char' is declared here
unsigned char *h2, unsigned char h3, ...)
                               ^
Specifically, it seems that both the C90 (4.8.1.1) and C11 (7.16.1.4)
standards explicitly call this out as undefined behavior:

The parameter parmN is the identifier of the rightmost parameter in
the variable parameter list in the function definition (the one just
before the ...). If the parameter parmN is declared with ... or with a
type that is not compatible with the type that results after
application of the default argument promotions, the behavior is
undefined.

Link: ClangBuiltLinux/linux#41
Link: https://www.eskimo.com/~scs/cclass/int/sx11c.html
Suggested-by: David Laight <David.Laight@aculab.com>
Suggested-by: Denis Kenzior <denkenz@gmail.com>
Suggested-by: James Bottomley <jejb@linux.vnet.ibm.com>
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: James Morris <james.morris@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
jason77-wang pushed a commit to jason77-wang/oem that referenced this issue Jul 5, 2019
BugLink: https://bugs.launchpad.net/bugs/1834529

[ Upstream commit be24b37e22c20cbaa891971616784dd0f35211e8 ]

Fixes the warning reported by Clang:
security/keys/trusted.c:146:17: warning: passing an object that
undergoes default
      argument promotion to 'va_start' has undefined behavior [-Wvarargs]
        va_start(argp, h3);
                       ^
security/keys/trusted.c:126:37: note: parameter of type 'unsigned
char' is declared here
unsigned char *h2, unsigned char h3, ...)
                               ^
Specifically, it seems that both the C90 (4.8.1.1) and C11 (7.16.1.4)
standards explicitly call this out as undefined behavior:

The parameter parmN is the identifier of the rightmost parameter in
the variable parameter list in the function definition (the one just
before the ...). If the parameter parmN is declared with ... or with a
type that is not compatible with the type that results after
application of the default argument promotions, the behavior is
undefined.

Link: ClangBuiltLinux/linux#41
Link: https://www.eskimo.com/~scs/cclass/int/sx11c.html
Suggested-by: David Laight <David.Laight@aculab.com>
Suggested-by: Denis Kenzior <denkenz@gmail.com>
Suggested-by: James Bottomley <jejb@linux.vnet.ibm.com>
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: James Morris <james.morris@microsoft.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Signed-off-by: Connor Kuehl <connor.kuehl@canonical.com>
Signed-off-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
nathanchance pushed a commit that referenced this issue Jul 17, 2019
Add Synopsys PCIe Endpoint eDMA IP core driver to kernel.

This IP is generally distributed with Synopsys PCIe Endpoint IP (depends
of the use and licensing agreement).

This core driver, initializes and configures the eDMA IP using vma-helpers
functions and dma-engine subsystem.

This driver can be compile as built-in or external module in kernel.

To enable this driver just select DW_EDMA option in kernel configuration,
however it requires and selects automatically DMA_ENGINE and
DMA_VIRTUAL_CHANNELS option too.

In order to transfer data from point A to B as fast as possible this IP
requires a dedicated memory space containing linked list of elements.

All elements of this linked list are continuous and each one describes a
data transfer (source and destination addresses, length and a control
variable).

For the sake of simplicity, lets assume a memory space for channel write
0 which allows about 42 elements.

+---------+
| Desc #0 |-+
+---------+ |
            V
       +----------+
       | Chunk #0 |-+
       |  CB = 1  | |  +----------+  +-----+  +-----------+  +-----+
       +----------+ +->| Burst #0 |->| ... |->| Burst #41 |->| llp |
            |          +----------+  +-----+  +-----------+  +-----+
            V
       +----------+
       | Chunk #1 |-+
       |  CB = 0  | |  +-----------+  +-----+  +-----------+  +-----+
       +----------+ +->| Burst #42 |->| ... |->| Burst #83 |->| llp |
            |          +-----------+  +-----+  +-----------+  +-----+
            V
       +----------+
       | Chunk #2 |-+
       |  CB = 1  | |  +-----------+  +-----+  +------------+  +-----+
       +----------+ +->| Burst #84 |->| ... |->| Burst #125 |->| llp |
            |          +-----------+  +-----+  +------------+  +-----+
            V
       +----------+
       | Chunk #3 |-+
       |  CB = 0  | |  +------------+  +-----+  +------------+  +-----+
       +----------+ +->| Burst #126 |->| ... |->| Burst #129 |->| llp |
                       +------------+  +-----+  +------------+  +-----+

Legend:
 - Linked list, also know as Chunk
 - Linked list element*, also know as Burst *CB*, also know as Change Bit,
it's a control bit (and typically is toggled) that allows to easily
identify and differentiate between the current linked list and the
previous or the next one.
 - LLP, is a special element that indicates the end of the linked list
element stream also informs that the next CB should be toggle

On every last Burst of the Chunk (Burst #41, Burst #83, Burst #125 or
even Burst #129) is set some flags on their control variable (RIE and
LIE bits) that will trigger the send of "done" interruption.

On the interruptions callback, is decided whether to recycle the linked
list memory space by writing a new set of Bursts elements (if still
exists Chunks to transfer) or is considered completed (if there is no
Chunks available to transfer).

On scatter-gather transfer mode, the client will submit a scatter-gather
list of n (on this case 130) elements, that will be divide in multiple
Chunks, each Chunk will have (on this case 42) a limited number of
Bursts and after transferring all Bursts, an interrupt will be
triggered, which will allow to recycle the all linked list dedicated
memory again with the new information relative to the next Chunk and
respective Burst associated and repeat the whole cycle again.

On cyclic transfer mode, the client will submit a buffer pointer, length
of it and number of repetitions, in this case each burst will correspond
directly to each repetition.

Each Burst can describes a data transfer from point A(source) to point
B(destination) with a length that can be from 1 byte up to 4 GB. Since
dedicated the memory space where the linked list will reside is limited,
the whole n burst elements will be organized in several Chunks, that
will be used later to recycle the dedicated memory space to initiate a
new sequence of data transfers.

The whole transfer is considered has completed when it was transferred
all bursts.

Currently this IP has a set well-known register map, which includes
support for legacy and unroll modes. Legacy mode is version of this
register map that has multiplexer register that allows to switch
registers between all write and read channels and the unroll modes
repeats all write and read channels registers with an offset between
them. This register map is called v0.

The IP team is creating a new register map more suitable to the latest
PCIe features, that very likely will change the map register, which this
version will be called v1. As soon as this new version is released by
the IP team the support for this version in be included on this driver.

According to the logic, patches 1, 2 and 3 should be squashed into 1
unique patch, but for the sake of simplicity of review, it was divided
in this 3 patches files.

Signed-off-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Cc: Vinod Koul <vkoul@kernel.org>
Cc: Dan Williams <dan.j.williams@intel.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>
Cc: Joao Pinto <jpinto@synopsys.com>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
nathanchance pushed a commit that referenced this issue Aug 7, 2019
There is a potential deadlock in rxrpc_peer_keepalive_dispatch() whereby
rxrpc_put_peer() is called with the peer_hash_lock held, but if it reduces
the peer's refcount to 0, rxrpc_put_peer() calls __rxrpc_put_peer() - which
the tries to take the already held lock.

Fix this by providing a version of rxrpc_put_peer() that can be called in
situations where the lock is already held.

The bug may produce the following lockdep report:

============================================
WARNING: possible recursive locking detected
5.2.0-next-20190718 #41 Not tainted
--------------------------------------------
kworker/0:3/21678 is trying to acquire lock:
00000000aa5eecdf (&(&rxnet->peer_hash_lock)->rlock){+.-.}, at: spin_lock_bh
/./include/linux/spinlock.h:343 [inline]
00000000aa5eecdf (&(&rxnet->peer_hash_lock)->rlock){+.-.}, at:
__rxrpc_put_peer /net/rxrpc/peer_object.c:415 [inline]
00000000aa5eecdf (&(&rxnet->peer_hash_lock)->rlock){+.-.}, at:
rxrpc_put_peer+0x2d3/0x6a0 /net/rxrpc/peer_object.c:435

but task is already holding lock:
00000000aa5eecdf (&(&rxnet->peer_hash_lock)->rlock){+.-.}, at: spin_lock_bh
/./include/linux/spinlock.h:343 [inline]
00000000aa5eecdf (&(&rxnet->peer_hash_lock)->rlock){+.-.}, at:
rxrpc_peer_keepalive_dispatch /net/rxrpc/peer_event.c:378 [inline]
00000000aa5eecdf (&(&rxnet->peer_hash_lock)->rlock){+.-.}, at:
rxrpc_peer_keepalive_worker+0x6b3/0xd02 /net/rxrpc/peer_event.c:430

Fixes: 330bdcf ("rxrpc: Fix the keepalive generator [ver #2]")
Reported-by: syzbot+72af434e4b3417318f84@syzkaller.appspotmail.com
Signed-off-by: David Howells <dhowells@redhat.com>
Reviewed-by: Marc Dionne <marc.dionne@auristor.com>
Reviewed-by: Jeffrey Altman <jaltman@auristor.com>
nathanchance pushed a commit that referenced this issue Dec 22, 2019
…A memory with different size"

The TI CPSW(s) driver produces warning with DMA API debug options enabled:

WARNING: CPU: 0 PID: 1033 at kernel/dma/debug.c:1025 check_unmap+0x4a8/0x968
DMA-API: cpsw 48484000.ethernet: device driver frees DMA memory with different size
 [device address=0x00000000abc6aa02] [map size=64 bytes] [unmap size=42 bytes]
CPU: 0 PID: 1033 Comm: ping Not tainted 5.3.0-dirty #41
Hardware name: Generic DRA72X (Flattened Device Tree)
[<c0112c60>] (unwind_backtrace) from [<c010d270>] (show_stack+0x10/0x14)
[<c010d270>] (show_stack) from [<c09bc564>] (dump_stack+0xd8/0x110)
[<c09bc564>] (dump_stack) from [<c013b93c>] (__warn+0xe0/0x10c)
[<c013b93c>] (__warn) from [<c013b9ac>] (warn_slowpath_fmt+0x44/0x6c)
[<c013b9ac>] (warn_slowpath_fmt) from [<c01e0368>] (check_unmap+0x4a8/0x968)
[<c01e0368>] (check_unmap) from [<c01e08a8>] (debug_dma_unmap_page+0x80/0x90)
[<c01e08a8>] (debug_dma_unmap_page) from [<c0752414>] (__cpdma_chan_free+0x114/0x16c)
[<c0752414>] (__cpdma_chan_free) from [<c07525c4>] (__cpdma_chan_process+0x158/0x17c)
[<c07525c4>] (__cpdma_chan_process) from [<c0753690>] (cpdma_chan_process+0x3c/0x5c)
[<c0753690>] (cpdma_chan_process) from [<c0758660>] (cpsw_tx_mq_poll+0x48/0x94)
[<c0758660>] (cpsw_tx_mq_poll) from [<c0803018>] (net_rx_action+0x108/0x4e4)
[<c0803018>] (net_rx_action) from [<c010230c>] (__do_softirq+0xec/0x598)
[<c010230c>] (__do_softirq) from [<c0143914>] (do_softirq.part.4+0x68/0x74)
[<c0143914>] (do_softirq.part.4) from [<c0143a44>] (__local_bh_enable_ip+0x124/0x17c)
[<c0143a44>] (__local_bh_enable_ip) from [<c0871590>] (ip_finish_output2+0x294/0xb7c)
[<c0871590>] (ip_finish_output2) from [<c0875440>] (ip_output+0x210/0x364)
[<c0875440>] (ip_output) from [<c0875e2c>] (ip_send_skb+0x1c/0xf8)
[<c0875e2c>] (ip_send_skb) from [<c08a7fd4>] (raw_sendmsg+0x9a8/0xc74)
[<c08a7fd4>] (raw_sendmsg) from [<c07d6b90>] (sock_sendmsg+0x14/0x24)
[<c07d6b90>] (sock_sendmsg) from [<c07d8260>] (__sys_sendto+0xbc/0x100)
[<c07d8260>] (__sys_sendto) from [<c01011ac>] (__sys_trace_return+0x0/0x14)
Exception stack(0xea9a7fa8 to 0xea9a7ff0)
...

The reason is that cpdma_chan_submit_si() now stores original buffer length
(sw_len) in CPDMA descriptor instead of adjusted buffer length (hw_len)
used to map the buffer.

Hence, fix an issue by passing correct buffer length in CPDMA descriptor.

Cc: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Fixes: 6670aca ("net: ethernet: ti: davinci_cpdma: add dma mapped submit")
Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Reviewed-by: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
nathanchance pushed a commit that referenced this issue Jul 18, 2020
ib_pd is accessed internally during destroy of the TIR/TIS, but PD
can be not set yet. This leading to the following kernel panic.

  BUG: kernel NULL pointer dereference, address: 0000000000000074
  PGD 8000000079eaa067 P4D 8000000079eaa067 PUD 7ae81067 PMD 0 Oops: 0000 [#1] SMP PTI
  CPU: 1 PID: 709 Comm: syz-executor.0 Not tainted 5.8.0-rc3 #41 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.12.1-0-ga5cab58e9a3f-prebuilt.qemu.org 04/01/2014
  RIP: 0010:destroy_raw_packet_qp_tis drivers/infiniband/hw/mlx5/qp.c:1189 [inline]
  RIP: 0010:destroy_raw_packet_qp drivers/infiniband/hw/mlx5/qp.c:1527 [inline]
  RIP: 0010:destroy_qp_common+0x2ca/0x4f0 drivers/infiniband/hw/mlx5/qp.c:2397
  Code: 00 85 c0 74 2e e8 56 18 55 ff 48 8d b3 28 01 00 00 48 89 ef e8 d7 d3 ff ff 48 8b 43 08 8b b3 c0 01 00 00 48 8b bd a8 0a 00 00 <0f> b7 50 74 e8 0d 6a fe ff e8 28 18 55 ff 49 8d 55 50 4c 89 f1 48
  RSP: 0018:ffffc900007bbac8 EFLAGS: 00010293
  RAX: 0000000000000000 RBX: ffff88807949e800 RCX: 0000000000000998
  RDX: 0000000000000000 RSI: 0000000000000008 RDI: ffff88807c180140
  RBP: ffff88807b50c000 R08: 000000000002d379 R09: ffffc900007bba00
  R10: 0000000000000001 R11: 000000000002d358 R12: ffff888076f37000
  R13: ffff88807949e9c8 R14: ffffc900007bbe08 R15: ffff888076f37000
  FS:  00000000019bf940(0000) GS:ffff88807dd00000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 0000000000000074 CR3: 0000000076d68004 CR4: 0000000000360ee0
  DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
  DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
  Call Trace:
   mlx5_ib_create_qp+0xf36/0xf90 drivers/infiniband/hw/mlx5/qp.c:3014
   _ib_create_qp drivers/infiniband/core/core_priv.h:333 [inline]
   create_qp+0x57f/0xd20 drivers/infiniband/core/uverbs_cmd.c:1443
   ib_uverbs_create_qp+0xcf/0x100 drivers/infiniband/core/uverbs_cmd.c:1564
   ib_uverbs_write+0x5fa/0x780 drivers/infiniband/core/uverbs_main.c:664
   __vfs_write+0x3f/0x90 fs/read_write.c:495
   vfs_write+0xc7/0x1f0 fs/read_write.c:559
   ksys_write+0x5e/0x110 fs/read_write.c:612
   do_syscall_64+0x3e/0x70 arch/x86/entry/common.c:359
   entry_SYSCALL_64_after_hwframe+0x44/0xa9
  RIP: 0033:0x466479
  Code: Bad RIP value.
  RSP: 002b:00007ffd057b62b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
  RAX: ffffffffffffffda RBX: 000000000073bf00 RCX: 0000000000466479
  RDX: 0000000000000070 RSI: 0000000020000240 RDI: 0000000000000003
  RBP: 00000000019bf8fc R08: 0000000000000000 R09: 0000000000000000
  R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
  R13: 0000000000000bf6 R14: 00000000004cb859 R15: 00000000006fefc0

Fixes: 6c41965 ("RDMA/mlx5: Don't access ib_qp fields in internal destroy QP path")
Link: https://lore.kernel.org/r/20200707110612.882962-4-leon@kernel.org
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
nathanchance pushed a commit that referenced this issue Jul 31, 2020
Fix kernel oops observed when an ext adv data is larger than 31 bytes.

This can be reproduced by setting up an advertiser with advertisement
larger than 31 bytes.  The issue is not sensitive to the advertisement
content.  In particular, this was reproduced with an advertisement of
229 bytes filled with 'A'.  See stack trace below.

This is fixed by not catching ext_adv as legacy adv are only cached to
be able to concatenate a scanable adv with its scan response before
sending it up through mgmt.

With ext_adv, this is no longer necessary.

  general protection fault: 0000 [#1] SMP PTI
  CPU: 6 PID: 205 Comm: kworker/u17:0 Not tainted 5.4.0-37-generic #41-Ubuntu
  Hardware name: Dell Inc. XPS 15 7590/0CF6RR, BIOS 1.7.0 05/11/2020
  Workqueue: hci0 hci_rx_work [bluetooth]
  RIP: 0010:hci_bdaddr_list_lookup+0x1e/0x40 [bluetooth]
  Code: ff ff e9 26 ff ff ff 0f 1f 44 00 00 0f 1f 44 00 00 55 48 8b 07 48 89 e5 48 39 c7 75 0a eb 24 48 8b 00 48 39 f8 74 1c 44 8b 06 <44> 39 40 10 75 ef 44 0f b7 4e 04 66 44 39 48 14 75 e3 38 50 16 75
  RSP: 0018:ffffbc6a40493c70 EFLAGS: 00010286
  RAX: 4141414141414141 RBX: 000000000000001b RCX: 0000000000000000
  RDX: 0000000000000000 RSI: ffff9903e76c100f RDI: ffff9904289d4b28
  RBP: ffffbc6a40493c70 R08: 0000000093570362 R09: 0000000000000000
  R10: 0000000000000000 R11: ffff9904344eae38 R12: ffff9904289d4000
  R13: 0000000000000000 R14: 00000000ffffffa3 R15: ffff9903e76c100f
  FS: 0000000000000000(0000) GS:ffff990434580000(0000) knlGS:0000000000000000
  CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 00007feed125a000 CR3: 00000001b860a003 CR4: 00000000003606e0
  Call Trace:
    process_adv_report+0x12e/0x560 [bluetooth]
    hci_le_meta_evt+0x7b2/0xba0 [bluetooth]
    hci_event_packet+0x1c29/0x2a90 [bluetooth]
    hci_rx_work+0x19b/0x360 [bluetooth]
    process_one_work+0x1eb/0x3b0
    worker_thread+0x4d/0x400
    kthread+0x104/0x140

Fixes: c215e93 ("Bluetooth: Process extended ADV report event")
Reported-by: Andy Nguyen <theflow@google.com>
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Reported-by: Balakrishna Godavarthi <bgodavar@codeaurora.org>
Signed-off-by: Alain Michaud <alainm@chromium.org>
Tested-by: Sonny Sasaka <sonnysasaka@chromium.org>
Acked-by: Marcel Holtmann <marcel@holtmann.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-Wvarargs [BUG] linux A bug that should be fixed in the mainline kernel. [FIXED][LINUX] 5.1 This bug was fixed in Linux 5.1
Projects
None yet
Development

No branches or pull requests

3 participants