diff mbox series

PCI: dwc: fix find_next_bit() usage

Message ID 20190904160339.2800-1-niklas.cassel@linaro.org
State Accepted
Commit 1137e61dcb99f7f8b54e77ed83f68b5b485a3e34
Headers show
Series PCI: dwc: fix find_next_bit() usage | expand

Commit Message

Niklas Cassel Sept. 4, 2019, 4:03 p.m. UTC 
find_next_bit() takes a parameter of size long, and performs arithmetic
that assumes that the argument is of size long.

Therefore we cannot pass a u32, since this will cause find_next_bit()
to read outside the stack buffer and will produce the following print:
BUG: KASAN: stack-out-of-bounds in find_next_bit+0x38/0xb0

Fixes: 1b497e6493c4 ("PCI: dwc: Fix uninitialized variable in dw_handle_msi_irq()")
Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>

---
 drivers/pci/controller/dwc/pcie-designware-host.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

-- 
2.21.0

Comments

Gustavo Pimentel Sept. 12, 2019, 11:31 a.m. UTC | #1 
On Wed, Sep 4, 2019 at 17:3:38, Niklas Cassel <niklas.cassel@linaro.org> 
wrote:

> find_next_bit() takes a parameter of size long, and performs arithmetic

> that assumes that the argument is of size long.

> 

> Therefore we cannot pass a u32, since this will cause find_next_bit()

> to read outside the stack buffer and will produce the following print:

> BUG: KASAN: stack-out-of-bounds in find_next_bit+0x38/0xb0

> 

> Fixes: 1b497e6493c4 ("PCI: dwc: Fix uninitialized variable in dw_handle_msi_irq()")

> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>

> ---

>  drivers/pci/controller/dwc/pcie-designware-host.c | 11 ++++++-----

>  1 file changed, 6 insertions(+), 5 deletions(-)

> 

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c

> index d3156446ff27..45f21640c977 100644

> --- a/drivers/pci/controller/dwc/pcie-designware-host.c

> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c

> @@ -78,7 +78,8 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {

>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)

>  {

>  	int i, pos, irq;

> -	u32 val, num_ctrls;

> +	unsigned long val;

> +	u32 status, num_ctrls;

>  	irqreturn_t ret = IRQ_NONE;

>  

>  	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;

> @@ -86,14 +87,14 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)

>  	for (i = 0; i < num_ctrls; i++) {

>  		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS +

>  					(i * MSI_REG_CTRL_BLOCK_SIZE),

> -				    4, &val);

> -		if (!val)

> +				    4, &status);

> +		if (!status)

>  			continue;

>  

>  		ret = IRQ_HANDLED;

> +		val = status;

>  		pos = 0;

> -		while ((pos = find_next_bit((unsigned long *) &val,

> -					    MAX_MSI_IRQS_PER_CTRL,

> +		while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,

>  					    pos)) != MAX_MSI_IRQS_PER_CTRL) {

>  			irq = irq_find_mapping(pp->irq_domain,

>  					       (i * MAX_MSI_IRQS_PER_CTRL) +

> -- 

> 2.21.0


Hi Niklas!

The patch looks nice! Thanks!

Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
Andrew Murray Sept. 12, 2019, 1:04 p.m. UTC | #2 
On Wed, Sep 04, 2019 at 06:03:38PM +0200, Niklas Cassel wrote:
> find_next_bit() takes a parameter of size long, and performs arithmetic

> that assumes that the argument is of size long.

> 

> Therefore we cannot pass a u32, since this will cause find_next_bit()

> to read outside the stack buffer and will produce the following print:

> BUG: KASAN: stack-out-of-bounds in find_next_bit+0x38/0xb0

> 

> Fixes: 1b497e6493c4 ("PCI: dwc: Fix uninitialized variable in dw_handle_msi_irq()")

> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>

> ---


Reviewed-by: Andrew Murray <andrew.murray@arm.com>


>  drivers/pci/controller/dwc/pcie-designware-host.c | 11 ++++++-----

>  1 file changed, 6 insertions(+), 5 deletions(-)

> 

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c

> index d3156446ff27..45f21640c977 100644

> --- a/drivers/pci/controller/dwc/pcie-designware-host.c

> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c

> @@ -78,7 +78,8 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {

>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)

>  {

>  	int i, pos, irq;

> -	u32 val, num_ctrls;

> +	unsigned long val;

> +	u32 status, num_ctrls;

>  	irqreturn_t ret = IRQ_NONE;

>  

>  	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;

> @@ -86,14 +87,14 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)

>  	for (i = 0; i < num_ctrls; i++) {

>  		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS +

>  					(i * MSI_REG_CTRL_BLOCK_SIZE),

> -				    4, &val);

> -		if (!val)

> +				    4, &status);

> +		if (!status)

>  			continue;

>  

>  		ret = IRQ_HANDLED;

> +		val = status;

>  		pos = 0;

> -		while ((pos = find_next_bit((unsigned long *) &val,

> -					    MAX_MSI_IRQS_PER_CTRL,

> +		while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,

>  					    pos)) != MAX_MSI_IRQS_PER_CTRL) {

>  			irq = irq_find_mapping(pp->irq_domain,

>  					       (i * MAX_MSI_IRQS_PER_CTRL) +

> -- 

> 2.21.0

>
Bjorn Andersson Sept. 13, 2019, 9:57 p.m. UTC | #3 
On Wed, Sep 4, 2019 at 9:03 AM Niklas Cassel <niklas.cassel@linaro.org> wrote:
>

> find_next_bit() takes a parameter of size long, and performs arithmetic

> that assumes that the argument is of size long.

>

> Therefore we cannot pass a u32, since this will cause find_next_bit()

> to read outside the stack buffer and will produce the following print:

> BUG: KASAN: stack-out-of-bounds in find_next_bit+0x38/0xb0

>

> Fixes: 1b497e6493c4 ("PCI: dwc: Fix uninitialized variable in dw_handle_msi_irq()")

> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>


Tested-by: Bjorn Andersson <bjorn.andersson@linaro.org>


> ---

>  drivers/pci/controller/dwc/pcie-designware-host.c | 11 ++++++-----

>  1 file changed, 6 insertions(+), 5 deletions(-)

>

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c

> index d3156446ff27..45f21640c977 100644

> --- a/drivers/pci/controller/dwc/pcie-designware-host.c

> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c

> @@ -78,7 +78,8 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {

>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)

>  {

>         int i, pos, irq;

> -       u32 val, num_ctrls;

> +       unsigned long val;

> +       u32 status, num_ctrls;

>         irqreturn_t ret = IRQ_NONE;

>

>         num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;

> @@ -86,14 +87,14 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)

>         for (i = 0; i < num_ctrls; i++) {

>                 dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS +

>                                         (i * MSI_REG_CTRL_BLOCK_SIZE),

> -                                   4, &val);

> -               if (!val)

> +                                   4, &status);

> +               if (!status)

>                         continue;

>

>                 ret = IRQ_HANDLED;

> +               val = status;

>                 pos = 0;

> -               while ((pos = find_next_bit((unsigned long *) &val,

> -                                           MAX_MSI_IRQS_PER_CTRL,

> +               while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,

>                                             pos)) != MAX_MSI_IRQS_PER_CTRL) {

>                         irq = irq_find_mapping(pp->irq_domain,

>                                                (i * MAX_MSI_IRQS_PER_CTRL) +

> --

> 2.21.0

>
Lorenzo Pieralisi Oct. 15, 2019, 3:34 p.m. UTC | #4 
On Wed, Sep 04, 2019 at 06:03:38PM +0200, Niklas Cassel wrote:
> find_next_bit() takes a parameter of size long, and performs arithmetic

> that assumes that the argument is of size long.

> 

> Therefore we cannot pass a u32, since this will cause find_next_bit()

> to read outside the stack buffer and will produce the following print:

> BUG: KASAN: stack-out-of-bounds in find_next_bit+0x38/0xb0

> 

> Fixes: 1b497e6493c4 ("PCI: dwc: Fix uninitialized variable in dw_handle_msi_irq()")

> Signed-off-by: Niklas Cassel <niklas.cassel@linaro.org>

> ---

>  drivers/pci/controller/dwc/pcie-designware-host.c | 11 ++++++-----

>  1 file changed, 6 insertions(+), 5 deletions(-)


Applied to pci/dwc, thanks.

Lorenzo

> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c

> index d3156446ff27..45f21640c977 100644

> --- a/drivers/pci/controller/dwc/pcie-designware-host.c

> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c

> @@ -78,7 +78,8 @@ static struct msi_domain_info dw_pcie_msi_domain_info = {

>  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)

>  {

>  	int i, pos, irq;

> -	u32 val, num_ctrls;

> +	unsigned long val;

> +	u32 status, num_ctrls;

>  	irqreturn_t ret = IRQ_NONE;

>  

>  	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;

> @@ -86,14 +87,14 @@ irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)

>  	for (i = 0; i < num_ctrls; i++) {

>  		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS +

>  					(i * MSI_REG_CTRL_BLOCK_SIZE),

> -				    4, &val);

> -		if (!val)

> +				    4, &status);

> +		if (!status)

>  			continue;

>  

>  		ret = IRQ_HANDLED;

> +		val = status;

>  		pos = 0;

> -		while ((pos = find_next_bit((unsigned long *) &val,

> -					    MAX_MSI_IRQS_PER_CTRL,

> +		while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,

>  					    pos)) != MAX_MSI_IRQS_PER_CTRL) {

>  			irq = irq_find_mapping(pp->irq_domain,

>  					       (i * MAX_MSI_IRQS_PER_CTRL) +

> -- 

> 2.21.0

>
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index d3156446ff27..45f21640c977 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -78,7 +78,8 @@  static struct msi_domain_info dw_pcie_msi_domain_info = {
 irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 {
 	int i, pos, irq;
-	u32 val, num_ctrls;
+	unsigned long val;
+	u32 status, num_ctrls;
 	irqreturn_t ret = IRQ_NONE;
 
 	num_ctrls = pp->num_vectors / MAX_MSI_IRQS_PER_CTRL;
@@ -86,14 +87,14 @@  irqreturn_t dw_handle_msi_irq(struct pcie_port *pp)
 	for (i = 0; i < num_ctrls; i++) {
 		dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS +
 					(i * MSI_REG_CTRL_BLOCK_SIZE),
-				    4, &val);
-		if (!val)
+				    4, &status);
+		if (!status)
 			continue;
 
 		ret = IRQ_HANDLED;
+		val = status;
 		pos = 0;
-		while ((pos = find_next_bit((unsigned long *) &val,
-					    MAX_MSI_IRQS_PER_CTRL,
+		while ((pos = find_next_bit(&val, MAX_MSI_IRQS_PER_CTRL,
 					    pos)) != MAX_MSI_IRQS_PER_CTRL) {
 			irq = irq_find_mapping(pp->irq_domain,
 					       (i * MAX_MSI_IRQS_PER_CTRL) +