[PATCH] nvme: rdma: Add check for queue in nvmet_rdma_cm_handler()

List overview All Threads
Download

newer

older

Re: Patch "lib:...

Passenger traffic 2024

George Rurikov

31 Oct 2024 31 Oct '24

5:33 p.m.

From: MrRurikov grurikov@gmal.com

After having been assigned to a NULL value at rdma.c:1758, pointer 'queue' is passed as 1st parameter in call to function 'nvmet_rdma_queue_established' at rdma.c:1773, as 1st parameter in call to function 'nvmet_rdma_queue_disconnect' at rdma.c:1787 and as 2nd parameter in call to function 'nvmet_rdma_queue_connect_fail' at rdma.c:1800, where it is dereferenced.

I understand, that driver is confident that the RDMA_CM_EVENT_CONNECT_REQUEST event will occur first and perform initialization, but maliciously prepared hardware could send events in violation of the protocol. Nothing guarantees that the sequence of events will start with RDMA_CM_EVENT_CONNECT_REQUEST.

Found by Linux Verification Center (linuxtesting.org) with SVACE

Fixes: e1a2ee249b19 ("nvmet-rdma: Fix use after free in nvmet_rdma_cm_handler()") Cc: stable@vger.kernel.org Signed-off-by: George Rurikov g.ryurikov@securitycode.ru --- drivers/nvme/target/rdma.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c index 1b6264fa5803..becebc95f349 100644 --- a/drivers/nvme/target/rdma.c +++ b/drivers/nvme/target/rdma.c @@ -1770,8 +1770,10 @@ static int nvmet_rdma_cm_handler(struct rdma_cm_id *cm_id, ret = nvmet_rdma_queue_connect(cm_id, event); break; case RDMA_CM_EVENT_ESTABLISHED: - nvmet_rdma_queue_established(queue); - break; + if (!queue) { + nvmet_rdma_queue_established(queue); + break; + } case RDMA_CM_EVENT_ADDR_CHANGE: if (!queue) { struct nvmet_rdma_port *port = cm_id->context; @@ -1782,8 +1784,10 @@ static int nvmet_rdma_cm_handler(struct rdma_cm_id *cm_id, fallthrough; case RDMA_CM_EVENT_DISCONNECTED: case RDMA_CM_EVENT_TIMEWAIT_EXIT: - nvmet_rdma_queue_disconnect(queue); - break; + if (!queue) { + nvmet_rdma_queue_disconnect(queue); + break; + } case RDMA_CM_EVENT_DEVICE_REMOVAL: ret = nvmet_rdma_device_removal(cm_id, queue); break; @@ -1793,8 +1797,10 @@ static int nvmet_rdma_cm_handler(struct rdma_cm_id *cm_id, fallthrough; case RDMA_CM_EVENT_UNREACHABLE: case RDMA_CM_EVENT_CONNECT_ERROR: - nvmet_rdma_queue_connect_fail(cm_id, queue); - break; + if (!queue) { + nvmet_rdma_queue_connect_fail(cm_id, queue); + break; + } default: pr_err("received unrecognized RDMA CM event %d\n", event->event);

-- 2.34.1

Show replies by date

Jens Axboe

31 Oct 31 Oct

5:41 p.m.

...

diff --git a/drivers/nvme/target/rdma.c b/drivers/nvme/target/rdma.c index 1b6264fa5803..becebc95f349 100644 --- a/drivers/nvme/target/rdma.c +++ b/drivers/nvme/target/rdma.c @@ -1770,8 +1770,10 @@ static int nvmet_rdma_cm_handler(struct rdma_cm_id *cm_id, ret = nvmet_rdma_queue_connect(cm_id, event); break; case RDMA_CM_EVENT_ESTABLISHED:
nvmet_rdma_queue_established(queue);
break;
if (!queue) {
	nvmet_rdma_queue_established(queue);
	break;
}

This, and the other hunks, just look like nonsense. Why on earth verify that the queue is NULL, then not use NULL after that. Let alone that whatever you pass it into happily dereference it, and now you've also got fallthrough errors all over the place.

This needs to go back to the drawing board. I'd worry a lot more about bad code than "potentially malicious hardware", to be honest.

-- Jens Axboe

kernel test robot

1 Nov 1 Nov

2:02 p.m.

Hi George,

kernel test robot noticed the following build warnings:

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

url: https://github.com/intel-lab-lkp/linux/commits/George-Rurikov/nvme-rdma-Add-... base: linus/master patch link: https://lore.kernel.org/r/20241031173327.663-1-grurikov%40gmail.com patch subject: [PATCH] nvme: rdma: Add check for queue in nvmet_rdma_cm_handler() config: x86_64-rhel-8.3-func (https://download.01.org/0day-ci/archive/20241101/202411012136.hoMlvrTF-lkp@i...) compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241101/202411012136.hoMlvrTF-lkp@i...)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202411012136.hoMlvrTF-lkp@intel.com/

All warnings (new ones prefixed by >>):

drivers/nvme/target/rdma.c: In function 'nvmet_rdma_cm_handler':

...

...
drivers/nvme/target/rdma.c:1771:20: warning: this statement may fall through [-Wimplicit-fallthrough=]

1771 | if (!queue) { | ^ drivers/nvme/target/rdma.c:1775:9: note: here 1775 | case RDMA_CM_EVENT_ADDR_CHANGE: | ^~~~ drivers/nvme/target/rdma.c:1785:20: warning: this statement may fall through [-Wimplicit-fallthrough=] 1785 | if (!queue) { | ^ drivers/nvme/target/rdma.c:1789:9: note: here 1789 | case RDMA_CM_EVENT_DEVICE_REMOVAL: | ^~~~ drivers/nvme/target/rdma.c:1798:20: warning: this statement may fall through [-Wimplicit-fallthrough=] 1798 | if (!queue) { | ^ drivers/nvme/target/rdma.c:1802:9: note: here 1802 | default: | ^~~~~~~

vim +1771 drivers/nvme/target/rdma.c

1752 1753 static int nvmet_rdma_cm_handler(struct rdma_cm_id *cm_id, 1754 struct rdma_cm_event *event) 1755 { 1756 struct nvmet_rdma_queue *queue = NULL; 1757 int ret = 0; 1758 1759 if (cm_id->qp) 1760 queue = cm_id->qp->qp_context; 1761 1762 pr_debug("%s (%d): status %d id %p\n", 1763 rdma_event_msg(event->event), event->event, 1764 event->status, cm_id); 1765 1766 switch (event->event) { 1767 case RDMA_CM_EVENT_CONNECT_REQUEST: 1768 ret = nvmet_rdma_queue_connect(cm_id, event); 1769 break; 1770 case RDMA_CM_EVENT_ESTABLISHED:

...

1771 if (!queue) {

1772 nvmet_rdma_queue_established(queue); 1773 break; 1774 } 1775 case RDMA_CM_EVENT_ADDR_CHANGE: 1776 if (!queue) { 1777 struct nvmet_rdma_port *port = cm_id->context; 1778 1779 queue_delayed_work(nvmet_wq, &port->repair_work, 0); 1780 break; 1781 } 1782 fallthrough; 1783 case RDMA_CM_EVENT_DISCONNECTED: 1784 case RDMA_CM_EVENT_TIMEWAIT_EXIT: 1785 if (!queue) { 1786 nvmet_rdma_queue_disconnect(queue); 1787 break; 1788 } 1789 case RDMA_CM_EVENT_DEVICE_REMOVAL: 1790 ret = nvmet_rdma_device_removal(cm_id, queue); 1791 break; 1792 case RDMA_CM_EVENT_REJECTED: 1793 pr_debug("Connection rejected: %s\n", 1794 rdma_reject_msg(cm_id, event->status)); 1795 fallthrough; 1796 case RDMA_CM_EVENT_UNREACHABLE: 1797 case RDMA_CM_EVENT_CONNECT_ERROR: 1798 if (!queue) { 1799 nvmet_rdma_queue_connect_fail(cm_id, queue); 1800 break; 1801 } 1802 default: 1803 pr_err("received unrecognized RDMA CM event %d\n", 1804 event->event); 1805 break; 1806 } 1807 1808 return ret; 1809 } 1810

-- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

kernel test robot

3:14 p.m.

Hi George,

kernel test robot noticed the following build warnings:

url: https://github.com/intel-lab-lkp/linux/commits/George-Rurikov/nvme-rdma-Add-... base: linus/master patch link: https://lore.kernel.org/r/20241031173327.663-1-grurikov%40gmail.com patch subject: [PATCH] nvme: rdma: Add check for queue in nvmet_rdma_cm_handler() config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20241101/202411012252.e1ChG1dy-lkp@i...) compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 639a7ac648f1e50ccd2556e17d401c04f9cce625) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241101/202411012252.e1ChG1dy-lkp@i...)

All warnings (new ones prefixed by >>):

In file included from drivers/nvme/target/rdma.c:8: In file included from include/linux/blk-integrity.h:5: In file included from include/linux/blk-mq.h:5: In file included from include/linux/blkdev.h:9: In file included from include/linux/blk_types.h:10: In file included from include/linux/bvec.h:10: In file included from include/linux/highmem.h:10: In file included from include/linux/mm.h:2213: include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion] 504 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS + | ~~~~~~~~~~~~~~~~~~~~~ ^ 505 | item]; | ~~~~ include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion] 511 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS + | ~~~~~~~~~~~~~~~~~~~~~ ^ 512 | NR_VM_NUMA_EVENT_ITEMS + | ~~~~~~~~~~~~~~~~~~~~~~ include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion] 518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_" | ~~~~~~~~~~~ ^ ~~~ include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion] 524 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS + | ~~~~~~~~~~~~~~~~~~~~~ ^ 525 | NR_VM_NUMA_EVENT_ITEMS + | ~~~~~~~~~~~~~~~~~~~~~~ In file included from drivers/nvme/target/rdma.c:8: In file included from include/linux/blk-integrity.h:5: In file included from include/linux/blk-mq.h:8: In file included from include/linux/scatterlist.h:9: In file included from arch/s390/include/asm/io.h:95: include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 548 | val = __raw_readb(PCI_IOBASE + addr); | ~~~~~~~~~~ ^ include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr)); | ~~~~~~~~~~ ^ include/uapi/linux/byteorder/big_endian.h:37:59: note: expanded from macro '__le16_to_cpu' 37 | #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x)) | ^ include/uapi/linux/swab.h:102:54: note: expanded from macro '__swab16' 102 | #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x)) | ^ In file included from drivers/nvme/target/rdma.c:8: In file included from include/linux/blk-integrity.h:5: In file included from include/linux/blk-mq.h:8: In file included from include/linux/scatterlist.h:9: In file included from arch/s390/include/asm/io.h:95: include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr)); | ~~~~~~~~~~ ^ include/uapi/linux/byteorder/big_endian.h:35:59: note: expanded from macro '__le32_to_cpu' 35 | #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x)) | ^ include/uapi/linux/swab.h:115:54: note: expanded from macro '__swab32' 115 | #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x)) | ^ In file included from drivers/nvme/target/rdma.c:8: In file included from include/linux/blk-integrity.h:5: In file included from include/linux/blk-mq.h:8: In file included from include/linux/scatterlist.h:9: In file included from arch/s390/include/asm/io.h:95: include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 585 | __raw_writeb(value, PCI_IOBASE + addr); | ~~~~~~~~~~ ^ include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr); | ~~~~~~~~~~ ^ include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr); | ~~~~~~~~~~ ^ include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 693 | readsb(PCI_IOBASE + addr, buffer, count); | ~~~~~~~~~~ ^ include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 701 | readsw(PCI_IOBASE + addr, buffer, count); | ~~~~~~~~~~ ^ include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 709 | readsl(PCI_IOBASE + addr, buffer, count); | ~~~~~~~~~~ ^ include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 718 | writesb(PCI_IOBASE + addr, buffer, count); | ~~~~~~~~~~ ^ include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 727 | writesw(PCI_IOBASE + addr, buffer, count); | ~~~~~~~~~~ ^ include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic] 736 | writesl(PCI_IOBASE + addr, buffer, count); | ~~~~~~~~~~ ^

...

...
drivers/nvme/target/rdma.c:1775:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]

1775 | case RDMA_CM_EVENT_ADDR_CHANGE: | ^ drivers/nvme/target/rdma.c:1775:2: note: insert '__attribute__((fallthrough));' to silence this warning 1775 | case RDMA_CM_EVENT_ADDR_CHANGE: | ^ | __attribute__((fallthrough)); drivers/nvme/target/rdma.c:1775:2: note: insert 'break;' to avoid fall-through 1775 | case RDMA_CM_EVENT_ADDR_CHANGE: | ^ | break; drivers/nvme/target/rdma.c:1789:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough] 1789 | case RDMA_CM_EVENT_DEVICE_REMOVAL: | ^ drivers/nvme/target/rdma.c:1789:2: note: insert '__attribute__((fallthrough));' to silence this warning 1789 | case RDMA_CM_EVENT_DEVICE_REMOVAL: | ^ | __attribute__((fallthrough)); drivers/nvme/target/rdma.c:1789:2: note: insert 'break;' to avoid fall-through 1789 | case RDMA_CM_EVENT_DEVICE_REMOVAL: | ^ | break; drivers/nvme/target/rdma.c:1802:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough] 1802 | default: | ^ drivers/nvme/target/rdma.c:1802:2: note: insert '__attribute__((fallthrough));' to silence this warning 1802 | default: | ^ | __attribute__((fallthrough)); drivers/nvme/target/rdma.c:1802:2: note: insert 'break;' to avoid fall-through 1802 | default: | ^ | break; 19 warnings generated.

vim +1775 drivers/nvme/target/rdma.c

d8f7750a08968b Sagi Grimberg 2016-05-19 1752 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1753 static int nvmet_rdma_cm_handler(struct rdma_cm_id *cm_id, 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1754 struct rdma_cm_event *event) 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1755 { 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1756 struct nvmet_rdma_queue *queue = NULL; 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1757 int ret = 0; 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1758 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1759 if (cm_id->qp) 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1760 queue = cm_id->qp->qp_context; 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1761 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1762 pr_debug("%s (%d): status %d id %p\n", 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1763 rdma_event_msg(event->event), event->event, 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1764 event->status, cm_id); 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1765 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1766 switch (event->event) { 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1767 case RDMA_CM_EVENT_CONNECT_REQUEST: 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1768 ret = nvmet_rdma_queue_connect(cm_id, event); 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1769 break; 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1770 case RDMA_CM_EVENT_ESTABLISHED: 554d31ea0a6334 MrRurikov 2024-10-31 1771 if (!queue) { 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1772 nvmet_rdma_queue_established(queue); 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1773 break; 554d31ea0a6334 MrRurikov 2024-10-31 1774 } 8f000cac6e7a6e Christoph Hellwig 2016-07-06 @1775 case RDMA_CM_EVENT_ADDR_CHANGE: a032e4f6d60d0a Sagi Grimberg 2020-04-02 1776 if (!queue) { a032e4f6d60d0a Sagi Grimberg 2020-04-02 1777 struct nvmet_rdma_port *port = cm_id->context; a032e4f6d60d0a Sagi Grimberg 2020-04-02 1778 8832cf922151e9 Sagi Grimberg 2022-03-21 1779 queue_delayed_work(nvmet_wq, &port->repair_work, 0); a032e4f6d60d0a Sagi Grimberg 2020-04-02 1780 break; a032e4f6d60d0a Sagi Grimberg 2020-04-02 1781 } df561f6688fef7 Gustavo A. R. Silva 2020-08-23 1782 fallthrough; 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1783 case RDMA_CM_EVENT_DISCONNECTED: 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1784 case RDMA_CM_EVENT_TIMEWAIT_EXIT: 554d31ea0a6334 MrRurikov 2024-10-31 1785 if (!queue) { 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1786 nvmet_rdma_queue_disconnect(queue); d8f7750a08968b Sagi Grimberg 2016-05-19 1787 break; 554d31ea0a6334 MrRurikov 2024-10-31 1788 } d8f7750a08968b Sagi Grimberg 2016-05-19 1789 case RDMA_CM_EVENT_DEVICE_REMOVAL: d8f7750a08968b Sagi Grimberg 2016-05-19 1790 ret = nvmet_rdma_device_removal(cm_id, queue); 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1791 break; 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1792 case RDMA_CM_EVENT_REJECTED: 512fb1b32bac02 Steve Wise 2016-10-26 1793 pr_debug("Connection rejected: %s\n", 512fb1b32bac02 Steve Wise 2016-10-26 1794 rdma_reject_msg(cm_id, event->status)); df561f6688fef7 Gustavo A. R. Silva 2020-08-23 1795 fallthrough; 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1796 case RDMA_CM_EVENT_UNREACHABLE: 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1797 case RDMA_CM_EVENT_CONNECT_ERROR: 554d31ea0a6334 MrRurikov 2024-10-31 1798 if (!queue) { 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1799 nvmet_rdma_queue_connect_fail(cm_id, queue); 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1800 break; 554d31ea0a6334 MrRurikov 2024-10-31 1801 } 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1802 default: 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1803 pr_err("received unrecognized RDMA CM event %d\n", 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1804 event->event); 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1805 break; 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1806 } 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1807 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1808 return ret; 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1809 } 8f000cac6e7a6e Christoph Hellwig 2016-07-06 1810

-- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki

days inactive

days old

linux-stable-mirror@lists.linaro.org

3 comments

participants

tags (0)

participants (3)

George Rurikov
Jens Axboe
kernel test robot