Skip to contentSkip to conversation

usb: gadget: fotg210-udc: Fix memory leak of fotg210->ep[i]

PATCH New Sasha Levin 1 participant 13 messages Oct 1, 2018

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/13 usb: gadget: fotg210-udc: Fix memory leak of fotg210->ep[i]

From: Anton Vasilyev <[email protected]>

[ Upstream commit c37bd52836296ecc9a0fc8060b819089aebdbcde ]

There is no deallocation of fotg210->ep[i] elements, allocated at fotg210_udc_probe.

The patch adds deallocation of fotg210->ep array elements and simplifies error path of fotg210_udc_probe().

Found by Linux Driver Verification project (linuxtesting.org).

---
 drivers/usb/gadget/udc/fotg210-udc.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)
drivers/usb/gadget/udc/fotg210-udc.c +10 -5
index 1d315921bf34..9500e2b6cd59 100644
@@ -1077,12 +1077,15 @@ static struct usb_gadget_ops fotg210_gadget_ops = {
 static int fotg210_udc_remove(struct platform_device *pdev)
 {
 	struct fotg210_udc *fotg210 = platform_get_drvdata(pdev);
+	int i;
 
 	usb_del_gadget_udc(&fotg210->gadget);
 	iounmap(fotg210->reg);
 	free_irq(platform_get_irq(pdev, 0), fotg210);
 
 	fotg210_ep_free_request(&fotg210->ep[0]->ep, fotg210->ep0_req);
+	for (i = 0; i < FOTG210_MAX_NUM_EP; i++)
+		kfree(fotg210->ep[i]);
 	kfree(fotg210);
 
 	return 0;
@@ -1113,7 +1116,7 @@ static int fotg210_udc_probe(struct platform_device *pdev)
 	/* initialize udc */
 	fotg210 = kzalloc(sizeof(struct fotg210_udc), GFP_KERNEL);
 	if (fotg210 == NULL)
-		goto err_alloc;
+		goto err;
 
 	for (i = 0; i < FOTG210_MAX_NUM_EP; i++) {
 		_ep[i] = kzalloc(sizeof(struct fotg210_ep), GFP_KERNEL);
@@ -1125,7 +1128,7 @@ static int fotg210_udc_probe(struct platform_device *pdev)
 	fotg210->reg = ioremap(res->start, resource_size(res));
 	if (fotg210->reg == NULL) {
 		pr_err("ioremap error.\n");
-		goto err_map;
+		goto err_alloc;
 	}
 
 	spin_lock_init(&fotg210->lock);
@@ -1162,7 +1165,7 @@ static int fotg210_udc_probe(struct platform_device *pdev)
 	fotg210->ep0_req = fotg210_ep_alloc_request(&fotg210->ep[0]->ep,
 				GFP_KERNEL);
 	if (fotg210->ep0_req == NULL)
-		goto err_req;
+		goto err_map;
 
 	fotg210_init(fotg210);
 
@@ -1191,12 +1194,14 @@ static int fotg210_udc_probe(struct platform_device *pdev)
 	fotg210_ep_free_request(&fotg210->ep[0]->ep, fotg210->ep0_req);
 
 err_map:
-	if (fotg210->reg)
-		iounmap(fotg210->reg);
+	iounmap(fotg210->reg);
 
 err_alloc:
+	for (i = 0; i < FOTG210_MAX_NUM_EP; i++)
+		kfree(fotg210->ep[i]);
 	kfree(fotg210);
 
+err:
 	return ret;
 }
 
Signature · 1 line
2.17.1
#1 lore raw

PATCH 2/13 pinctrl: msm: Really mask level interrupts to prevent latching

From: Stephen Boyd <[email protected]>

[ Upstream commit b55326dc969ea2d704a008d9a97583b128f54f4f ]

The interrupt controller hardware in this pin controller has two status enable bits. The first "normal" status enable bit enables or disables the summary interrupt line being raised when a gpio interrupt triggers and the "raw" status enable bit allows or prevents the hardware from latching an interrupt into the status register for a gpio interrupt. Currently we just toggle the "normal" status enable bit in the mask and unmask ops so that the summary irq interrupt going to the CPU's interrupt controller doesn't trigger for the masked gpio interrupt.

For a level triggered interrupt, the flow would be as follows: the pin controller sees the interrupt, latches the status into the status register, raises the summary irq to the CPU, summary irq handler runs and calls handle_level_irq(), handle_level_irq() masks and acks the gpio interrupt, the interrupt handler runs, and finally unmask the interrupt. When the interrupt handler completes, we expect that the interrupt line level will go back to the deasserted state so the genirq code can unmask the interrupt without it triggering again.

If we only mask the interrupt by clearing the "normal" status enable bit then we'll ack the interrupt but it will continue to show up as pending in the status register because the raw status bit is enabled, the hardware hasn't deasserted the line, and thus the asserted state latches into the status register again. When the hardware deasserts the interrupt the pin controller still thinks there is a pending unserviced level interrupt because it latched it earlier. This behavior causes software to see an extra interrupt for level type interrupts each time the interrupt is handled.

Let's fix this by clearing the raw status enable bit for level type interrupts so that the hardware stops latching the status of the interrupt after we ack it. We don't do this for edge type interrupts because it seems that toggling the raw status enable bit for edge type interrupts causes spurious edge interrupts.

---
 drivers/pinctrl/qcom/pinctrl-msm.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
drivers/pinctrl/qcom/pinctrl-msm.c +24 -0
index db4c22dd07fa..9824e36ad7b9 100644
@@ -596,6 +596,29 @@ static void msm_gpio_irq_mask(struct irq_data *d)
 	spin_lock_irqsave(&pctrl->lock, flags);
 
 	val = readl(pctrl->regs + g->intr_cfg_reg);
+	/*
+	 * There are two bits that control interrupt forwarding to the CPU. The
+	 * RAW_STATUS_EN bit causes the level or edge sensed on the line to be
+	 * latched into the interrupt status register when the hardware detects
+	 * an irq that it's configured for (either edge for edge type or level
+	 * for level type irq). The 'non-raw' status enable bit causes the
+	 * hardware to assert the summary interrupt to the CPU if the latched
+	 * status bit is set. There's a bug though, the edge detection logic
+	 * seems to have a problem where toggling the RAW_STATUS_EN bit may
+	 * cause the status bit to latch spuriously when there isn't any edge
+	 * so we can't touch that bit for edge type irqs and we have to keep
+	 * the bit set anyway so that edges are latched while the line is masked.
+	 *
+	 * To make matters more complicated, leaving the RAW_STATUS_EN bit
+	 * enabled all the time causes level interrupts to re-latch into the
+	 * status register because the level is still present on the line after
+	 * we ack it. We clear the raw status enable bit during mask here and
+	 * set the bit on unmask so the interrupt can't latch into the hardware
+	 * while it's masked.
+	 */
+	if (irqd_get_trigger_type(d) & IRQ_TYPE_LEVEL_MASK)
+		val &= ~BIT(g->intr_raw_status_bit);
+
 	val &= ~BIT(g->intr_enable_bit);
 	writel(val, pctrl->regs + g->intr_cfg_reg);
 
@@ -617,6 +640,7 @@ static void msm_gpio_irq_unmask(struct irq_data *d)
 	spin_lock_irqsave(&pctrl->lock, flags);
 
 	val = readl(pctrl->regs + g->intr_cfg_reg);
+	val |= BIT(g->intr_raw_status_bit);
 	val |= BIT(g->intr_enable_bit);
 	writel(val, pctrl->regs + g->intr_cfg_reg);
 
Signature · 1 line
2.17.1
#2 lore raw

PATCH 3/13 RDMA/ucma: check fd type in ucma_migrate_id()

In reply to Sasha Levin

From: Jann Horn <[email protected]>

[ Upstream commit 0d23ba6034b9cf48b8918404367506da3e4b3ee5 ]

The current code grabs the private_data of whatever file descriptor userspace has supplied and implicitly casts it to a struct ucma_file *, potentially causing a type confusion.

This is probably fine in practice because the pointer is only used for comparisons, it is never actually dereferenced; and even in the comparisons, it is unlikely that a file from another filesystem would have a ->private_data pointer that happens to also be valid in this context. But ->private_data is not always guaranteed to be a valid pointer to an object owned by the file's filesystem; for example, some filesystems just cram numbers in there.

Check the type of the supplied file descriptor to be safe, analogous to how other places in the kernel do it.

Fixes 88314e4dda1e ("RDMA/cma: add support for rdma_migrate_id()")
---
 drivers/infiniband/core/ucma.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/infiniband/core/ucma.c +6 -0
index eaadd636c21b..a399d7da7774 100644
@@ -113,6 +113,8 @@ static DEFINE_MUTEX(mut);
 static DEFINE_IDR(ctx_idr);
 static DEFINE_IDR(multicast_idr);
 
+static const struct file_operations ucma_fops;
+
 static inline struct ucma_context *_ucma_find_context(int id,
 						      struct ucma_file *file)
 {
@@ -1416,6 +1418,10 @@ static ssize_t ucma_migrate_id(struct ucma_file *new_file,
 	f = fdget(cmd.fd);
 	if (!f.file)
 		return -ENOENT;
+	if (f.file->f_op != &ucma_fops) {
+		ret = -EINVAL;
+		goto file_put;
+	}
 
 	/* Validate current fd and prevent destruction of id. */
 	ctx = ucma_get_ctx(f.file->private_data, cmd.id);
Signature · 1 line
2.17.1
#3 lore raw

PATCH 4/13 USB: yurex: Check for truncation in yurex_read()

In reply to Sasha Levin

From: Ben Hutchings <[email protected]>

[ Upstream commit 14427b86837a4baf1c121934c6599bdb67dfa9fc ]

snprintf() always returns the full length of the string it could have printed, even if it was truncated because the buffer was too small. So in case the counter value is truncated, we will over-read from in_buffer and over-write to the caller's buffer.

I don't think it's actually possible for this to happen, but in case truncation occurs, WARN and return -EIO.

---
 drivers/usb/misc/yurex.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/misc/yurex.c +3 -0
index 83eaccfa9ee5..fbcb1cd4c118 100644
@@ -430,6 +430,9 @@ static ssize_t yurex_read(struct file *file, char *buffer, size_t count, loff_t
 	spin_unlock_irqrestore(&dev->lock, flags);
 	mutex_unlock(&dev->io_mutex);
 
+	if (WARN_ON_ONCE(len >= sizeof(in_buffer)))
+		return -EIO;
+
 	return simple_read_from_buffer(buffer, count, ppos, in_buffer, len);
 }
 
Signature · 1 line
2.17.1
#4 lore raw

PATCH 5/13 fs/cifs: suppress a string overflow warning

In reply to Sasha Levin

From: Stephen Rothwell <[email protected]>

[ Upstream commit bcfb84a996f6fa90b5e6e2954b2accb7a4711097 ]

A powerpc build of cifs with gcc v8.2.0 produces this warning:

fs/cifs/cifssmb.c: In function ‘CIFSSMBNegotiate’:
fs/cifs/cifssmb.c:605:3: warning: ‘strncpy’ writing 16 bytes into a region of size 1 overflows the destination [-Wstringop-overflow=]
strncpy(pSMB->DialectsArray+count, protocols[i].name, 16); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Since we are already doing a strlen() on the source, change the strncpy to a memcpy().

---
 fs/cifs/cifssmb.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)
fs/cifs/cifssmb.c +8 -3
index 949deb938dc9..db72c5284d13 100644
@@ -577,10 +577,15 @@ CIFSSMBNegotiate(const unsigned int xid, struct cifs_ses *ses)
 	}
 
 	count = 0;
+	/*
+	 * We know that all the name entries in the protocols array
+	 * are short (< 16 bytes anyway) and are NUL terminated.
+	 */
 	for (i = 0; i < CIFS_NUM_PROT; i++) {
-		strncpy(pSMB->DialectsArray+count, protocols[i].name, 16);
-		count += strlen(protocols[i].name) + 1;
-		/* null at end of source and target buffers anyway */
+		size_t len = strlen(protocols[i].name) + 1;
+
+		memcpy(pSMB->DialectsArray+count, protocols[i].name, len);
+		count += len;
 	}
 	inc_rfc1001_len(pSMB, count);
 	pSMB->ByteCount = cpu_to_le16(count);
Signature · 1 line
2.17.1
#5 lore raw

PATCH 6/13 dm thin metadata: try to avoid ever aborting transactions

In reply to Sasha Levin

From: Joe Thornber <[email protected]>

[ Upstream commit 3ab91828166895600efd9cdc3a0eb32001f7204a ]

Committing a transaction can consume some metadata of it's own, we now reserve a small amount of metadata to cover this. Free metadata reported by the kernel will not include this reserve.

If any of the reserve has been used after a commit we enter a new internal state PM_OUT_OF_METADATA_SPACE. This is reported as PM_READ_ONLY, so no userland changes are needed. If the metadata device is resized the pool will move back to PM_WRITE.

These changes mean we never need to abort and rollback a transaction due to running out of metadata space. This is particularly important because there have been a handful of reports of data corruption against DM thin-provisioning that can all be attributed to the thin-pool having ran out of metadata space.

---
 drivers/md/dm-thin-metadata.c | 36 ++++++++++++++++-
 drivers/md/dm-thin.c          | 73 +++++++++++++++++++++++++++++++----
 2 files changed, 100 insertions(+), 9 deletions(-)
drivers/md/dm-thin-metadata.c +35 -1
index 3f3ab7586dc0..096de9433da2 100644
@@ -190,6 +190,12 @@ struct dm_pool_metadata {
 	sector_t data_block_size;
 	bool read_only:1;
 
+	/*
+	 * We reserve a section of the metadata for commit overhead.
+	 * All reported space does *not* include this.
+	 */
+	dm_block_t metadata_reserve;
+
 	/*
 	 * Set if a transaction has to be aborted but the attempt to roll back
 	 * to the previous (good) transaction failed.  The only pool metadata
@@ -824,6 +830,22 @@ static int __commit_transaction(struct dm_pool_metadata *pmd)
 	return dm_tm_commit(pmd->tm, sblock);
 }
 
+static void __set_metadata_reserve(struct dm_pool_metadata *pmd)
+{
+	int r;
+	dm_block_t total;
+	dm_block_t max_blocks = 4096; /* 16M */
+
+	r = dm_sm_get_nr_blocks(pmd->metadata_sm, &total);
+	if (r) {
+		DMERR("could not get size of metadata device");
+		pmd->metadata_reserve = max_blocks;
+	} else {
+		sector_div(total, 10);
+		pmd->metadata_reserve = min(max_blocks, total);
+	}
+}
+
 struct dm_pool_metadata *dm_pool_metadata_open(struct block_device *bdev,
 					       sector_t data_block_size,
 					       bool format_device)
@@ -858,6 +880,8 @@ struct dm_pool_metadata *dm_pool_metadata_open(struct block_device *bdev,
 		return ERR_PTR(r);
 	}
 
+	__set_metadata_reserve(pmd);
+
 	return pmd;
 }
 
@@ -1625,6 +1649,13 @@ int dm_pool_get_free_metadata_block_count(struct dm_pool_metadata *pmd,
 	down_read(&pmd->root_lock);
 	if (!pmd->fail_io)
 		r = dm_sm_get_nr_free(pmd->metadata_sm, result);
+
+	if (!r) {
+		if (*result < pmd->metadata_reserve)
+			*result = 0;
+		else
+			*result -= pmd->metadata_reserve;
+	}
 	up_read(&pmd->root_lock);
 
 	return r;
@@ -1746,8 +1777,11 @@ int dm_pool_resize_metadata_dev(struct dm_pool_metadata *pmd, dm_block_t new_cou
 	int r = -EINVAL;
 
 	down_write(&pmd->root_lock);
-	if (!pmd->fail_io)
+	if (!pmd->fail_io) {
 		r = __resize_space_map(pmd->metadata_sm, new_count);
+		if (!r)
+			__set_metadata_reserve(pmd);
+	}
 	up_write(&pmd->root_lock);
 
 	return r;
drivers/md/dm-thin.c +65 -8

141 lines not on this page · raw message

Signature · 1 line
2.17.1
#6 lore raw

PATCH 7/13 arch/hexagon: fix kernel/dma.c build warning

In reply to Sasha Levin

From: Randy Dunlap <[email protected]>

[ Upstream commit 200f351e27f014fcbf69b544b0b4b72aeaf45fd3 ]

Fix build warning in arch/hexagon/kernel/dma.c by casting a void * to unsigned long to match the function parameter type.

../arch/hexagon/kernel/dma.c: In function 'arch_dma_alloc':
../arch/hexagon/kernel/dma.c:51:5: warning: passing argument 2 of 'gen_pool_add' makes integer from pointer without a cast [enabled by default] ../include/linux/genalloc.h:112:19: note: expected 'long unsigned int' but argument is of type 'void *'

Patch-mainline: linux-kernel @ 07/20/2018, 20:17

[[email protected]: fixed architecture name]
---
 arch/hexagon/kernel/dma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/hexagon/kernel/dma.c +1 -1
index b74f9bae31a3..5a821e1c4b9b 100644
@@ -79,7 +79,7 @@ static void *hexagon_dma_alloc_coherent(struct device *dev, size_t size,
 			panic("Can't create %s() memory pool!", __func__);
 		else
 			gen_pool_add(coherent_pool,
-				pfn_to_virt(max_low_pfn),
+				(unsigned long)pfn_to_virt(max_low_pfn),
 				hexagon_coherent_pool_size, -1);
 	}
 
Signature · 1 line
2.17.1
#7 lore raw

PATCH 8/13 hexagon: modify ffs() and fls() to return int

In reply to Sasha Levin

From: Randy Dunlap <[email protected]>

[ Upstream commit 5c41aaad409c097cf1ef74f2c649fed994744ef5 ]

Building drivers/mtd/nand/raw/nandsim.c on arch/hexagon/ produces a printk format build warning. This is due to hexagon's ffs() being coded as returning long instead of int.

Fix the printk format warning by changing all of hexagon's ffs() and fls() functions to return int instead of long. The variables that they return are already int instead of long. This return type matches the return type in <asm-generic/bitops/>.

../drivers/mtd/nand/raw/nandsim.c: In function 'init_nandsim':
../drivers/mtd/nand/raw/nandsim.c:760:2: warning: format '%u' expects argument of type 'unsigned int', but argument 2 has type 'long int' [-Wformat]

There are no ffs() or fls() allmodconfig build errors after making this change.

Cc Richard Kuo <[email protected]>
Cc Geert Uytterhoeven <[email protected]>

Patch-mainline: linux-kernel @ 07/22/2018, 16:03

---
 arch/hexagon/include/asm/bitops.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
arch/hexagon/include/asm/bitops.h +2 -2
index 5e4a59b3ec1b..2691a1857d20 100644
@@ -211,7 +211,7 @@ static inline long ffz(int x)
  * This is defined the same way as ffs.
  * Note fls(0) = 0, fls(1) = 1, fls(0x80000000) = 32.
  */
-static inline long fls(int x)
+static inline int fls(int x)
 {
 	int r;
 
@@ -232,7 +232,7 @@ static inline long fls(int x)
  * the libc and compiler builtin ffs routines, therefore
  * differs in spirit from the above ffz (man ffs).
  */
-static inline long ffs(int x)
+static inline int ffs(int x)
 {
 	int r;
 
Signature · 1 line
2.17.1
#8 lore raw

PATCH 9/13 r8169: Clear RTL_FLAG_TASK_*_PENDING when clearing RTL_FLAG_TASK_ENABLED

In reply to Sasha Levin

From: Kai-Heng Feng <[email protected]>

[ Upstream commit 6ad569019999300afd8e614d296fdc356550b77f ]

After system suspend, sometimes the r8169 doesn't work when ethernet cable gets pluggued.

This issue happens because rtl_reset_work() doesn't get called from rtl8169_runtime_resume(), after system suspend.

In rtl_task(), RTL_FLAG_TASK_* only gets cleared if this condition is met:
if (!netif_running(dev) ||

    !test_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags))
    ...

If RTL_FLAG_TASK_ENABLED was cleared during system suspend while RTL_FLAG_TASK_RESET_PENDING was set, the next rtl_schedule_task() won't schedule task as the flag is still there.

So in addition to clearing RTL_FLAG_TASK_ENABLED, also clears other flags.

---
 drivers/net/ethernet/realtek/r8169.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/net/ethernet/realtek/r8169.c +6 -3
index d1ea2dfbe16f..c9aa42ca2b64 100644
@@ -749,7 +749,7 @@ struct rtl8169_counters {
 };
 
 enum rtl_flag {
-	RTL_FLAG_TASK_ENABLED,
+	RTL_FLAG_TASK_ENABLED = 0,
 	RTL_FLAG_TASK_SLOW_PENDING,
 	RTL_FLAG_TASK_RESET_PENDING,
 	RTL_FLAG_TASK_PHY_PENDING,
@@ -7523,7 +7523,8 @@ static int rtl8169_close(struct net_device *dev)
 	rtl8169_update_counters(dev);
 
 	rtl_lock_work(tp);
-	clear_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags);
+	/* Clear all task flags */
+	bitmap_zero(tp->wk.flags, RTL_FLAG_MAX);
 
 	rtl8169_down(dev);
 	rtl_unlock_work(tp);
@@ -7679,7 +7680,9 @@ static void rtl8169_net_suspend(struct net_device *dev)
 
 	rtl_lock_work(tp);
 	napi_disable(&tp->napi);
-	clear_bit(RTL_FLAG_TASK_ENABLED, tp->wk.flags);
+	/* Clear all task flags */
+	bitmap_zero(tp->wk.flags, RTL_FLAG_MAX);
+
 	rtl_unlock_work(tp);
 
 	rtl_pll_power_down(tp);
Signature · 1 line
2.17.1
#9 lore raw

PATCH 10/13 s390/qeth: don't dump past end of unknown HW header

In reply to Sasha Levin

From: Julian Wiedmann <[email protected]>

[ Upstream commit 0ac1487c4b2de383b91ecad1be561b8f7a2c15f4 ]

For inbound data with an unsupported HW header format, only dump the actual HW header. We have no idea how much payload follows it, and what it contains. Worst case, we dump past the end of the Inbound Buffer and access whatever is located next in memory.

---
 drivers/s390/net/qeth_l2_main.c | 2 +-
 drivers/s390/net/qeth_l3_main.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
drivers/s390/net/qeth_l2_main.c +1 -1
index 1405fe4c0c96..da8d94b27ec9 100644
@@ -452,7 +452,7 @@ static int qeth_l2_process_inbound_buffer(struct qeth_card *card,
 		default:
 			dev_kfree_skb_any(skb);
 			QETH_CARD_TEXT(card, 3, "inbunkno");
-			QETH_DBF_HEX(CTRL, 3, hdr, QETH_DBF_CTRL_LEN);
+			QETH_DBF_HEX(CTRL, 3, hdr, sizeof(*hdr));
 			continue;
 		}
 		work_done++;
drivers/s390/net/qeth_l3_main.c +1 -1
index 908c294aebae..f3a765655072 100644
@@ -1993,7 +1993,7 @@ static int qeth_l3_process_inbound_buffer(struct qeth_card *card,
 		default:
 			dev_kfree_skb_any(skb);
 			QETH_CARD_TEXT(card, 3, "inbunkno");
-			QETH_DBF_HEX(CTRL, 3, hdr, QETH_DBF_CTRL_LEN);
+			QETH_DBF_HEX(CTRL, 3, hdr, sizeof(*hdr));
 			continue;
 		}
 		work_done++;
Signature · 1 line
2.17.1
#10 lore raw

PATCH 11/13 cifs: read overflow in is_valid_oplock_break()

In reply to Sasha Levin

From: Dan Carpenter <[email protected]>

[ Upstream commit 097f5863b1a0c9901f180bbd56ae7d630655faaa ]

We need to verify that the "data_offset" is within bounds.

---
 fs/cifs/misc.c | 8 ++++++++
 1 file changed, 8 insertions(+)
fs/cifs/misc.c +8 -0
index eedbc34e19db..c8662e212afc 100644
@@ -405,9 +405,17 @@ is_valid_oplock_break(char *buffer, struct TCP_Server_Info *srv)
 			(struct smb_com_transaction_change_notify_rsp *)buf;
 		struct file_notify_information *pnotify;
 		__u32 data_offset = 0;
+		size_t len = srv->total_read - sizeof(pSMBr->hdr.smb_buf_length);
+
 		if (get_bcc(buf) > sizeof(struct file_notify_information)) {
 			data_offset = le32_to_cpu(pSMBr->DataOffset);
 
+			if (data_offset >
+			    len - sizeof(struct file_notify_information)) {
+				cifs_dbg(FYI, "invalid data_offset %u\n",
+					 data_offset);
+				return true;
+			}
 			pnotify = (struct file_notify_information *)
 				((char *)&pSMBr->hdr.Protocol + data_offset);
 			cifs_dbg(FYI, "dnotify on %s Action: 0x%x\n",
Signature · 1 line
2.17.1
#11 lore raw

PATCH 12/13 xen/manage: don't complain about an empty value in control/sysrq node

In reply to Sasha Levin

From: Vitaly Kuznetsov <[email protected]>

[ Upstream commit 87dffe86d406bee8782cac2db035acb9a28620a7 ]

When guest receives a sysrq request from the host it acknowledges it by writing '\0' to control/sysrq xenstore node. This, however, make xenstore watch fire again but xenbus_scanf() fails to parse empty value with "%c" format string:

sysrq: SysRq : Emergency Sync
Emergency Sync complete
xen:manage: Error -34 reading sysrq code in control/sysrq

Ignore -ERANGE the same way we already ignore -ENOENT, empty value in control/sysrq is totally legal.

---
 drivers/xen/manage.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/xen/manage.c +4 -2
index 77909a2c6723..101189fbf032 100644
@@ -277,9 +277,11 @@ static void sysrq_handler(struct xenbus_watch *watch, const char **vec,
 		/*
 		 * The Xenstore watch fires directly after registering it and
 		 * after a suspend/resume cycle. So ENOENT is no error but
-		 * might happen in those cases.
+		 * might happen in those cases. ERANGE is observed when we get
+		 * an empty value (''), this happens when we acknowledge the
+		 * request by writing '\0' below.
 		 */
-		if (err != -ENOENT)
+		if (err != -ENOENT && err != -ERANGE)
 			pr_err("Error %d reading sysrq code in control/sysrq\n",
 			       err);
 		xenbus_transaction_end(xbt, 1);
Signature · 1 line
2.17.1
#12 lore raw

PATCH 13/13 xen: fix GCC warning and remove duplicate EVTCHN_ROW/EVTCHN_COL usage

In reply to Sasha Levin

From: Josh Abraham <[email protected]>

[ Upstream commit 4dca864b59dd150a221730775e2f21f49779c135 ]

This patch removes duplicate macro useage in events_base.c.

It also fixes gcc warning:
variable ‘col’ set but not used [-Wunused-but-set-variable]

---
 drivers/xen/events/events_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/xen/events/events_base.c +1 -1
index f7297217b83c..da6f2b6b726c 100644
@@ -138,7 +138,7 @@ static int set_evtchn_to_irq(unsigned evtchn, unsigned irq)
 		clear_evtchn_to_irq_row(row);
 	}
 
-	evtchn_to_irq[EVTCHN_ROW(evtchn)][EVTCHN_COL(evtchn)] = irq;
+	evtchn_to_irq[row][col] = irq;
 	return 0;
 }
 
Signature · 1 line
2.17.1