* [PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-02 16:06 ` David Dueck
0 siblings, 0 replies; 39+ messages in thread
From: David Dueck @ 2015-07-02 16:06 UTC (permalink / raw)
To: plagnioj
Cc: Nicolas Ferre, Alexandre Belloni, Ludovic Desroches,
Boris Brezillon, linux-arm-kernel, linux-kernel
Not all gpio banks are necessarily enabled, in the current code this can
lead to null pointer dereferences.
[ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
[ 51.130000] pgd = dee04000
[ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
[ 51.140000] Internal error: Oops: 17 [#1] ARM
[ 51.140000] Modules linked in:
[ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
[ 51.140000] Hardware name: Atmel SAMA5
[ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
[ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
[ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
[ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
sp : dec61e48 ip : 600f0013 fp : df522538
[ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
[ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
[ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
[ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
[ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
[ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
[ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
[ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
[ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
[ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
[ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
[ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
[ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
[ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
[ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
[ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
[ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
[ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
[ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
[ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
[ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
[ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
[ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
[ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
[ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
[ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
[ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
[ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
[ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
Signed-off-by: David Dueck <davidcdueck@googlemail.com>
CC: Nicolas Ferre <nicolas.ferre@atmel.com>
CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
CC: Ludovic Desroches <ludovic.desroches@atmel.com>
CC: Boris Brezillon <boris.brezillon@free-electrons.com>
CC: linux-arm-kernel@lists.infradead.org
CC: linux-kernel@vger.kernel.org
---
drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index a082447..2deb130 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
static void __iomem *pin_to_controller(struct at91_pinctrl *info,
unsigned int bank)
{
+ if (!gpio_chips[bank])
+ return NULL;
+
return gpio_chips[bank]->regbase;
}
@@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
pin = &pins_conf[i];
at91_pin_dbg(info->dev, pin);
pio = pin_to_controller(info, pin->bank);
+
+ if (!pio)
+ continue;
+
mask = pin_to_mask(pin->pin);
at91_mux_disable_interrupt(pio, mask);
switch (pin->mux) {
@@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
*config = 0;
dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+ if (!pio)
+ return -EINVAL;
+
pin = pin_id % MAX_NB_GPIO_PER_BANK;
if (at91_mux_get_multidrive(pio, pin))
@@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
"%s:%d, pin_id=%d, config=0x%lx",
__func__, __LINE__, pin_id, config);
pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+ if (!pio)
+ return -EINVAL;
+
pin = pin_id % MAX_NB_GPIO_PER_BANK;
mask = pin_to_mask(pin);
--
2.4.4
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-02 16:06 ` David Dueck
0 siblings, 0 replies; 39+ messages in thread
From: David Dueck @ 2015-07-02 16:06 UTC (permalink / raw)
To: linux-arm-kernel
Not all gpio banks are necessarily enabled, in the current code this can
lead to null pointer dereferences.
[ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
[ 51.130000] pgd = dee04000
[ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
[ 51.140000] Internal error: Oops: 17 [#1] ARM
[ 51.140000] Modules linked in:
[ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
[ 51.140000] Hardware name: Atmel SAMA5
[ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
[ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
[ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
[ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
sp : dec61e48 ip : 600f0013 fp : df522538
[ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
[ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
[ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
[ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
[ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
[ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
[ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
[ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
[ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
[ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
[ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
[ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
[ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
[ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
[ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
[ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
[ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
[ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
[ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
[ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
[ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
[ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
[ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
[ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
[ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
[ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
[ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
[ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
[ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
Signed-off-by: David Dueck <davidcdueck@googlemail.com>
CC: Nicolas Ferre <nicolas.ferre@atmel.com>
CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
CC: Ludovic Desroches <ludovic.desroches@atmel.com>
CC: Boris Brezillon <boris.brezillon@free-electrons.com>
CC: linux-arm-kernel at lists.infradead.org
CC: linux-kernel at vger.kernel.org
---
drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index a082447..2deb130 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
static void __iomem *pin_to_controller(struct at91_pinctrl *info,
unsigned int bank)
{
+ if (!gpio_chips[bank])
+ return NULL;
+
return gpio_chips[bank]->regbase;
}
@@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
pin = &pins_conf[i];
at91_pin_dbg(info->dev, pin);
pio = pin_to_controller(info, pin->bank);
+
+ if (!pio)
+ continue;
+
mask = pin_to_mask(pin->pin);
at91_mux_disable_interrupt(pio, mask);
switch (pin->mux) {
@@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
*config = 0;
dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+ if (!pio)
+ return -EINVAL;
+
pin = pin_id % MAX_NB_GPIO_PER_BANK;
if (at91_mux_get_multidrive(pio, pin))
@@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
"%s:%d, pin_id=%d, config=0x%lx",
__func__, __LINE__, pin_id, config);
pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+ if (!pio)
+ return -EINVAL;
+
pin = pin_id % MAX_NB_GPIO_PER_BANK;
mask = pin_to_mask(pin);
--
2.4.4
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [PATCH] pinctrl: at91: fix null pointer dereference
2015-07-02 16:06 ` David Dueck
(?)
@ 2015-07-03 9:38 ` Ludovic Desroches
-1 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-03 9:38 UTC (permalink / raw)
To: David Dueck
Cc: plagnioj, Nicolas Ferre, Alexandre Belloni, Ludovic Desroches,
Boris Brezillon, linux-arm-kernel, linux-kernel, linux-gpio
On Thu, Jul 02, 2015 at 06:06:42PM +0200, David Dueck wrote:
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> [ 51.130000] pgd = dee04000
> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> [ 51.140000] Internal error: Oops: 17 [#1] ARM
> [ 51.140000] Modules linked in:
> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> [ 51.140000] Hardware name: Atmel SAMA5
> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> sp : dec61e48 ip : 600f0013 fp : df522538
> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Thanks. I have added linux-gpio@vger.kernel.org to recipients.
> ---
> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a082447..2deb130 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> unsigned int bank)
> {
> + if (!gpio_chips[bank])
> + return NULL;
> +
> return gpio_chips[bank]->regbase;
> }
>
> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> pin = &pins_conf[i];
> at91_pin_dbg(info->dev, pin);
> pio = pin_to_controller(info, pin->bank);
> +
> + if (!pio)
> + continue;
> +
> mask = pin_to_mask(pin->pin);
> at91_mux_disable_interrupt(pio, mask);
> switch (pin->mux) {
> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> *config = 0;
> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
> if (at91_mux_get_multidrive(pio, pin))
> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> "%s:%d, pin_id=%d, config=0x%lx",
> __func__, __LINE__, pin_id, config);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
> mask = pin_to_mask(pin);
>
> --
> 2.4.4
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-03 9:38 ` Ludovic Desroches
0 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-03 9:38 UTC (permalink / raw)
To: David Dueck
Cc: plagnioj, Nicolas Ferre, Alexandre Belloni, Ludovic Desroches,
Boris Brezillon, linux-arm-kernel, linux-kernel, linux-gpio
On Thu, Jul 02, 2015 at 06:06:42PM +0200, David Dueck wrote:
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> [ 51.130000] pgd = dee04000
> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> [ 51.140000] Internal error: Oops: 17 [#1] ARM
> [ 51.140000] Modules linked in:
> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> [ 51.140000] Hardware name: Atmel SAMA5
> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> sp : dec61e48 ip : 600f0013 fp : df522538
> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Thanks. I have added linux-gpio@vger.kernel.org to recipients.
> ---
> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a082447..2deb130 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> unsigned int bank)
> {
> + if (!gpio_chips[bank])
> + return NULL;
> +
> return gpio_chips[bank]->regbase;
> }
>
> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> pin = &pins_conf[i];
> at91_pin_dbg(info->dev, pin);
> pio = pin_to_controller(info, pin->bank);
> +
> + if (!pio)
> + continue;
> +
> mask = pin_to_mask(pin->pin);
> at91_mux_disable_interrupt(pio, mask);
> switch (pin->mux) {
> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> *config = 0;
> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
> if (at91_mux_get_multidrive(pio, pin))
> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> "%s:%d, pin_id=%d, config=0x%lx",
> __func__, __LINE__, pin_id, config);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
> mask = pin_to_mask(pin);
>
> --
> 2.4.4
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-03 9:38 ` Ludovic Desroches
0 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-03 9:38 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jul 02, 2015 at 06:06:42PM +0200, David Dueck wrote:
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> [ 51.130000] pgd = dee04000
> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> [ 51.140000] Internal error: Oops: 17 [#1] ARM
> [ 51.140000] Modules linked in:
> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> [ 51.140000] Hardware name: Atmel SAMA5
> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> sp : dec61e48 ip : 600f0013 fp : df522538
> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-kernel at vger.kernel.org
Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Thanks. I have added linux-gpio at vger.kernel.org to recipients.
> ---
> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a082447..2deb130 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> unsigned int bank)
> {
> + if (!gpio_chips[bank])
> + return NULL;
> +
> return gpio_chips[bank]->regbase;
> }
>
> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> pin = &pins_conf[i];
> at91_pin_dbg(info->dev, pin);
> pio = pin_to_controller(info, pin->bank);
> +
> + if (!pio)
> + continue;
> +
> mask = pin_to_mask(pin->pin);
> at91_mux_disable_interrupt(pio, mask);
> switch (pin->mux) {
> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> *config = 0;
> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
> if (at91_mux_get_multidrive(pio, pin))
> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> "%s:%d, pin_id=%d, config=0x%lx",
> __func__, __LINE__, pin_id, config);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
> mask = pin_to_mask(pin);
>
> --
> 2.4.4
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] pinctrl: at91: fix null pointer dereference
2015-07-02 16:06 ` David Dueck
@ 2015-07-07 17:02 ` Alexandre Belloni
-1 siblings, 0 replies; 39+ messages in thread
From: Alexandre Belloni @ 2015-07-07 17:02 UTC (permalink / raw)
To: David Dueck
Cc: plagnioj, Nicolas Ferre, Ludovic Desroches, Boris Brezillon,
linux-arm-kernel, linux-kernel
On 02/07/2015 at 18:06:42 +0200, David Dueck wrote :
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
[...]
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-07 17:02 ` Alexandre Belloni
0 siblings, 0 replies; 39+ messages in thread
From: Alexandre Belloni @ 2015-07-07 17:02 UTC (permalink / raw)
To: linux-arm-kernel
On 02/07/2015 at 18:06:42 +0200, David Dueck wrote :
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
[...]
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-kernel at vger.kernel.org
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
--
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] pinctrl: at91: fix null pointer dereference
2015-07-02 16:06 ` David Dueck
@ 2015-07-13 6:14 ` Jean-Christophe PLAGNIOL-VILLARD
-1 siblings, 0 replies; 39+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2015-07-13 6:14 UTC (permalink / raw)
To: David Dueck
Cc: Jean-Christophe PLAGNIOL-VILLARD, Nicolas FERRE,
Alexandre Belloni, Ludovic Desroches, Boris Brezillon,
linux-arm-kernel, linux-kernel
> On Jul 3, 2015, at 12:06 AM, David Dueck <davidcdueck@googlemail.com> wrote:
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> [ 51.130000] pgd = dee04000
> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> [ 51.140000] Internal error: Oops: 17 [#1] ARM
> [ 51.140000] Modules linked in:
> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> [ 51.140000] Hardware name: Atmel SAMA5
> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> sp : dec61e48 ip : 600f0013 fp : df522538
> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> ---
> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a082447..2deb130 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> unsigned int bank)
> {
> + if (!gpio_chips[bank])
> + return NULL;
if the bank is not enabled you should never arrived here
> +
> return gpio_chips[bank]->regbase;
> }
>
> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> pin = &pins_conf[i];
> at91_pin_dbg(info->dev, pin);
> pio = pin_to_controller(info, pin->bank);
> +
> + if (!pio)
> + continue;
> +
or here
> mask = pin_to_mask(pin->pin);
> at91_mux_disable_interrupt(pio, mask);
> switch (pin->mux) {
> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> *config = 0;
> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
ditto
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
> if (at91_mux_get_multidrive(pio, pin))
> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> "%s:%d, pin_id=%d, config=0x%lx",
> __func__, __LINE__, pin_id, config);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
ditoo
NACK
the problem is somewhere else
Best Regards,
J.
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
> mask = pin_to_mask(pin);
>
> --
> 2.4.4
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-13 6:14 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 0 replies; 39+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2015-07-13 6:14 UTC (permalink / raw)
To: linux-arm-kernel
> On Jul 3, 2015, at 12:06 AM, David Dueck <davidcdueck@googlemail.com> wrote:
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> [ 51.130000] pgd = dee04000
> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> [ 51.140000] Internal error: Oops: 17 [#1] ARM
> [ 51.140000] Modules linked in:
> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> [ 51.140000] Hardware name: Atmel SAMA5
> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> sp : dec61e48 ip : 600f0013 fp : df522538
> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-kernel at vger.kernel.org
> ---
> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a082447..2deb130 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> unsigned int bank)
> {
> + if (!gpio_chips[bank])
> + return NULL;
if the bank is not enabled you should never arrived here
> +
> return gpio_chips[bank]->regbase;
> }
>
> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> pin = &pins_conf[i];
> at91_pin_dbg(info->dev, pin);
> pio = pin_to_controller(info, pin->bank);
> +
> + if (!pio)
> + continue;
> +
or here
> mask = pin_to_mask(pin->pin);
> at91_mux_disable_interrupt(pio, mask);
> switch (pin->mux) {
> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> *config = 0;
> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
ditto
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
> if (at91_mux_get_multidrive(pio, pin))
> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> "%s:%d, pin_id=%d, config=0x%lx",
> __func__, __LINE__, pin_id, config);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
ditoo
NACK
the problem is somewhere else
Best Regards,
J.
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
> mask = pin_to_mask(pin);
>
> --
> 2.4.4
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] pinctrl: at91: fix null pointer dereference
2015-07-13 6:14 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2015-07-13 13:00 ` Ludovic Desroches
-1 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-13 13:00 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: David Dueck, Nicolas FERRE, Alexandre Belloni, Ludovic Desroches,
Boris Brezillon, linux-arm-kernel, linux-kernel
On Mon, Jul 13, 2015 at 02:14:51PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> > On Jul 3, 2015, at 12:06 AM, David Dueck <davidcdueck@googlemail.com> wrote:
> >
> > Not all gpio banks are necessarily enabled, in the current code this can
> > lead to null pointer dereferences.
> >
> > [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> > [ 51.130000] pgd = dee04000
> > [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> > [ 51.140000] Internal error: Oops: 17 [#1] ARM
> > [ 51.140000] Modules linked in:
> > [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> > [ 51.140000] Hardware name: Atmel SAMA5
> > [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> > [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> > [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> > [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> > sp : dec61e48 ip : 600f0013 fp : df522538
> > [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> > [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> > [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> > [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> > [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> > [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> > [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> > [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> > [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> > [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> > [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> > [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> > [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> > [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> > [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> > [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> > [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> > [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> > [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> > [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> > [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> > [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> > [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> > [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> > [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> > [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> > [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> > [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> > [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
> >
> > Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> > CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> > CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> > CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> > CC: linux-arm-kernel@lists.infradead.org
> > CC: linux-kernel@vger.kernel.org
> > ---
> > drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> > index a082447..2deb130 100644
> > --- a/drivers/pinctrl/pinctrl-at91.c
> > +++ b/drivers/pinctrl/pinctrl-at91.c
> > @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> > static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> > unsigned int bank)
> > {
> > + if (!gpio_chips[bank])
> > + return NULL;
>
> if the bank is not enabled you should never arrived here
But you can because in pinconf_pins_show you have:
for (i = 0; i < pctldev->desc->npins; i++)
And in the pinctrl-at91 driver you have:
at91_pinctrl_desc.npins = gpio_banks * MAX_NB_GPIO_PER_BANK;
>
>
> > +
> > return gpio_chips[bank]->regbase;
> > }
> >
> > @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> > pin = &pins_conf[i];
> > at91_pin_dbg(info->dev, pin);
> > pio = pin_to_controller(info, pin->bank);
> > +
> > + if (!pio)
> > + continue;
> > +
>
> or here
> > mask = pin_to_mask(pin->pin);
> > at91_mux_disable_interrupt(pio, mask);
> > switch (pin->mux) {
> > @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> > *config = 0;
> > dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> > pio = pin_to_controller(info, pin_to_bank(pin_id));
> > +
> > + if (!pio)
> > + return -EINVAL;
> > +
> ditto
> > pin = pin_id % MAX_NB_GPIO_PER_BANK;
> >
> > if (at91_mux_get_multidrive(pio, pin))
> > @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> > "%s:%d, pin_id=%d, config=0x%lx",
> > __func__, __LINE__, pin_id, config);
> > pio = pin_to_controller(info, pin_to_bank(pin_id));
> > +
> > + if (!pio)
> > + return -EINVAL;
> > +
>
> ditoo
>
> NACK
>
> the problem is somewhere else
I agree that the root cause is somewhere else. We have a
'gpio_chips[MAX_GPIO_BANKS]' table and 'npins = gpio_banks *
MAX_NB_GPIO_PER_BANK' where 'gpio_banks = max(gpio_banks, alias_idx +
1)'. If you have a disabled controller, you will have a NULL pointer in
the gpio_chips table and some pins which can't be accessed (for example
because this pio controller is secure only).
This patch is a sanity check. It is a bit rude to nack this kind of
patch telling we should never get this situation. There is always some
cases we didn't think of. Moreover, I think this patch is in the view of
the driver: the device can manage up to 160 pins but for some reasons, only
128 can be managed by Linux.
>
> Best Regards,
> J.
> > pin = pin_id % MAX_NB_GPIO_PER_BANK;
> > mask = pin_to_mask(pin);
> >
> > --
> > 2.4.4
> >
>
Regards
Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-13 13:00 ` Ludovic Desroches
0 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-13 13:00 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jul 13, 2015 at 02:14:51PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> > On Jul 3, 2015, at 12:06 AM, David Dueck <davidcdueck@googlemail.com> wrote:
> >
> > Not all gpio banks are necessarily enabled, in the current code this can
> > lead to null pointer dereferences.
> >
> > [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> > [ 51.130000] pgd = dee04000
> > [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> > [ 51.140000] Internal error: Oops: 17 [#1] ARM
> > [ 51.140000] Modules linked in:
> > [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> > [ 51.140000] Hardware name: Atmel SAMA5
> > [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> > [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> > [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> > [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> > sp : dec61e48 ip : 600f0013 fp : df522538
> > [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> > [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> > [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> > [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> > [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> > [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> > [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> > [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> > [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> > [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> > [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> > [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> > [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> > [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> > [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> > [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> > [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> > [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> > [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> > [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> > [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> > [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> > [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> > [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> > [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> > [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> > [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> > [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> > [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> > [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
> >
> > Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> > CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> > CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> > CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> > CC: linux-arm-kernel at lists.infradead.org
> > CC: linux-kernel at vger.kernel.org
> > ---
> > drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> > 1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> > index a082447..2deb130 100644
> > --- a/drivers/pinctrl/pinctrl-at91.c
> > +++ b/drivers/pinctrl/pinctrl-at91.c
> > @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> > static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> > unsigned int bank)
> > {
> > + if (!gpio_chips[bank])
> > + return NULL;
>
> if the bank is not enabled you should never arrived here
But you can because in pinconf_pins_show you have:
for (i = 0; i < pctldev->desc->npins; i++)
And in the pinctrl-at91 driver you have:
at91_pinctrl_desc.npins = gpio_banks * MAX_NB_GPIO_PER_BANK;
>
>
> > +
> > return gpio_chips[bank]->regbase;
> > }
> >
> > @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> > pin = &pins_conf[i];
> > at91_pin_dbg(info->dev, pin);
> > pio = pin_to_controller(info, pin->bank);
> > +
> > + if (!pio)
> > + continue;
> > +
>
> or here
> > mask = pin_to_mask(pin->pin);
> > at91_mux_disable_interrupt(pio, mask);
> > switch (pin->mux) {
> > @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> > *config = 0;
> > dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> > pio = pin_to_controller(info, pin_to_bank(pin_id));
> > +
> > + if (!pio)
> > + return -EINVAL;
> > +
> ditto
> > pin = pin_id % MAX_NB_GPIO_PER_BANK;
> >
> > if (at91_mux_get_multidrive(pio, pin))
> > @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> > "%s:%d, pin_id=%d, config=0x%lx",
> > __func__, __LINE__, pin_id, config);
> > pio = pin_to_controller(info, pin_to_bank(pin_id));
> > +
> > + if (!pio)
> > + return -EINVAL;
> > +
>
> ditoo
>
> NACK
>
> the problem is somewhere else
I agree that the root cause is somewhere else. We have a
'gpio_chips[MAX_GPIO_BANKS]' table and 'npins = gpio_banks *
MAX_NB_GPIO_PER_BANK' where 'gpio_banks = max(gpio_banks, alias_idx +
1)'. If you have a disabled controller, you will have a NULL pointer in
the gpio_chips table and some pins which can't be accessed (for example
because this pio controller is secure only).
This patch is a sanity check. It is a bit rude to nack this kind of
patch telling we should never get this situation. There is always some
cases we didn't think of. Moreover, I think this patch is in the view of
the driver: the device can manage up to 160 pins but for some reasons, only
128 can be managed by Linux.
>
> Best Regards,
> J.
> > pin = pin_id % MAX_NB_GPIO_PER_BANK;
> > mask = pin_to_mask(pin);
> >
> > --
> > 2.4.4
> >
>
Regards
Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] pinctrl: at91: fix null pointer dereference
2015-07-13 13:00 ` Ludovic Desroches
@ 2015-07-13 20:30 ` Jean-Christophe PLAGNIOL-VILLARD
-1 siblings, 0 replies; 39+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2015-07-13 20:30 UTC (permalink / raw)
To: Ludovic Desroches
Cc: Jean-Christophe PLAGNIOL-VILLARD, David Dueck, Nicolas FERRE,
Alexandre Belloni, Boris Brezillon, linux-arm-kernel,
linux-kernel
> On Jul 13, 2015, at 9:00 PM, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
>
> On Mon, Jul 13, 2015 at 02:14:51PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>
>>> On Jul 3, 2015, at 12:06 AM, David Dueck <davidcdueck@googlemail.com> wrote:
>>>
>>> Not all gpio banks are necessarily enabled, in the current code this can
>>> lead to null pointer dereferences.
>>>
>>> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
>>> [ 51.130000] pgd = dee04000
>>> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
>>> [ 51.140000] Internal error: Oops: 17 [#1] ARM
>>> [ 51.140000] Modules linked in:
>>> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
>>> [ 51.140000] Hardware name: Atmel SAMA5
>>> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
>>> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
>>> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
>>> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
>>> sp : dec61e48 ip : 600f0013 fp : df522538
>>> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
>>> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
>>> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
>>> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
>>> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
>>> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
>>> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
>>> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
>>> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
>>> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
>>> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
>>> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
>>> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
>>> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
>>> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
>>> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
>>> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
>>> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
>>> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
>>> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
>>> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
>>> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
>>> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
>>> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
>>> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
>>> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
>>> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
>>> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
>>> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
>>> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
>>>
>>> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
>>> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
>>> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
>>> CC: linux-arm-kernel@lists.infradead.org
>>> CC: linux-kernel@vger.kernel.org
>>> ---
>>> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
>>> 1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>>> index a082447..2deb130 100644
>>> --- a/drivers/pinctrl/pinctrl-at91.c
>>> +++ b/drivers/pinctrl/pinctrl-at91.c
>>> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
>>> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
>>> unsigned int bank)
>>> {
>>> + if (!gpio_chips[bank])
>>> + return NULL;
>>
>> if the bank is not enabled you should never arrived here
>
> But you can because in pinconf_pins_show you have:
> for (i = 0; i < pctldev->desc->npins; i++)
>
> And in the pinctrl-at91 driver you have:
> at91_pinctrl_desc.npins = gpio_banks * MAX_NB_GPIO_PER_BANK;
>
>>
>>
>>> +
>>> return gpio_chips[bank]->regbase;
>>> }
>>>
>>> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
>>> pin = &pins_conf[i];
>>> at91_pin_dbg(info->dev, pin);
>>> pio = pin_to_controller(info, pin->bank);
>>> +
>>> + if (!pio)
>>> + continue;
>>> +
>>
>> or here
>>> mask = pin_to_mask(pin->pin);
>>> at91_mux_disable_interrupt(pio, mask);
>>> switch (pin->mux) {
>>> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
>>> *config = 0;
>>> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
>>> pio = pin_to_controller(info, pin_to_bank(pin_id));
>>> +
>>> + if (!pio)
>>> + return -EINVAL;
>>> +
>> ditto
>>> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>>>
>>> if (at91_mux_get_multidrive(pio, pin))
>>> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
>>> "%s:%d, pin_id=%d, config=0x%lx",
>>> __func__, __LINE__, pin_id, config);
>>> pio = pin_to_controller(info, pin_to_bank(pin_id));
>>> +
>>> + if (!pio)
>>> + return -EINVAL;
>>> +
>>
>> ditoo
>>
>> NACK
>>
>> the problem is somewhere else
>
> I agree that the root cause is somewhere else.
So we MUST fix the root and not do quick fix
> We have a
> 'gpio_chips[MAX_GPIO_BANKS]' table and 'npins = gpio_banks *
> MAX_NB_GPIO_PER_BANK' where 'gpio_banks = max(gpio_banks, alias_idx +
> 1)'. If you have a disabled controller, you will have a NULL pointer in
> the gpio_chips table and some pins which can't be accessed (for example
> because this pio controller is secure only).
I agree but you to catch in one point ONLY of the bank is enabled or not
not everywhere.
>
> This patch is a sanity check.
You need only one at the entry point if you do have a NULL pointer accessed by the code
path the kernel need to oops so you known there is a problem, specially in secure mode.
This could means a bug or worse.
And This will cause CPU cycle consuming for nothing.
> It is a bit rude to nack this kind of
> patch telling we should never get this situation. There is always some
> cases we didn't think of. Moreover, I think this patch is in the view of
> the driver: the device can manage up to 160 pins but for some reasons, only
> 128 can be managed by Linux.
This an other issue.
>
>
>>
>> Best Regards,
>> J.
>>> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>>> mask = pin_to_mask(pin);
>>>
>>> --
>>> 2.4.4
>>>
>>
>
> Regards
>
> Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-13 20:30 ` Jean-Christophe PLAGNIOL-VILLARD
0 siblings, 0 replies; 39+ messages in thread
From: Jean-Christophe PLAGNIOL-VILLARD @ 2015-07-13 20:30 UTC (permalink / raw)
To: linux-arm-kernel
> On Jul 13, 2015, at 9:00 PM, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
>
> On Mon, Jul 13, 2015 at 02:14:51PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
>>
>>> On Jul 3, 2015, at 12:06 AM, David Dueck <davidcdueck@googlemail.com> wrote:
>>>
>>> Not all gpio banks are necessarily enabled, in the current code this can
>>> lead to null pointer dereferences.
>>>
>>> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
>>> [ 51.130000] pgd = dee04000
>>> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
>>> [ 51.140000] Internal error: Oops: 17 [#1] ARM
>>> [ 51.140000] Modules linked in:
>>> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
>>> [ 51.140000] Hardware name: Atmel SAMA5
>>> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
>>> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
>>> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
>>> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
>>> sp : dec61e48 ip : 600f0013 fp : df522538
>>> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
>>> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
>>> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
>>> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
>>> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
>>> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
>>> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
>>> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
>>> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
>>> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
>>> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
>>> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
>>> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
>>> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
>>> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
>>> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
>>> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
>>> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
>>> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
>>> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
>>> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
>>> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
>>> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
>>> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
>>> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
>>> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
>>> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
>>> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
>>> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
>>> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
>>>
>>> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
>>> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
>>> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
>>> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
>>> CC: linux-arm-kernel at lists.infradead.org
>>> CC: linux-kernel at vger.kernel.org
>>> ---
>>> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
>>> 1 file changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
>>> index a082447..2deb130 100644
>>> --- a/drivers/pinctrl/pinctrl-at91.c
>>> +++ b/drivers/pinctrl/pinctrl-at91.c
>>> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
>>> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
>>> unsigned int bank)
>>> {
>>> + if (!gpio_chips[bank])
>>> + return NULL;
>>
>> if the bank is not enabled you should never arrived here
>
> But you can because in pinconf_pins_show you have:
> for (i = 0; i < pctldev->desc->npins; i++)
>
> And in the pinctrl-at91 driver you have:
> at91_pinctrl_desc.npins = gpio_banks * MAX_NB_GPIO_PER_BANK;
>
>>
>>
>>> +
>>> return gpio_chips[bank]->regbase;
>>> }
>>>
>>> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
>>> pin = &pins_conf[i];
>>> at91_pin_dbg(info->dev, pin);
>>> pio = pin_to_controller(info, pin->bank);
>>> +
>>> + if (!pio)
>>> + continue;
>>> +
>>
>> or here
>>> mask = pin_to_mask(pin->pin);
>>> at91_mux_disable_interrupt(pio, mask);
>>> switch (pin->mux) {
>>> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
>>> *config = 0;
>>> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
>>> pio = pin_to_controller(info, pin_to_bank(pin_id));
>>> +
>>> + if (!pio)
>>> + return -EINVAL;
>>> +
>> ditto
>>> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>>>
>>> if (at91_mux_get_multidrive(pio, pin))
>>> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
>>> "%s:%d, pin_id=%d, config=0x%lx",
>>> __func__, __LINE__, pin_id, config);
>>> pio = pin_to_controller(info, pin_to_bank(pin_id));
>>> +
>>> + if (!pio)
>>> + return -EINVAL;
>>> +
>>
>> ditoo
>>
>> NACK
>>
>> the problem is somewhere else
>
> I agree that the root cause is somewhere else.
So we MUST fix the root and not do quick fix
> We have a
> 'gpio_chips[MAX_GPIO_BANKS]' table and 'npins = gpio_banks *
> MAX_NB_GPIO_PER_BANK' where 'gpio_banks = max(gpio_banks, alias_idx +
> 1)'. If you have a disabled controller, you will have a NULL pointer in
> the gpio_chips table and some pins which can't be accessed (for example
> because this pio controller is secure only).
I agree but you to catch in one point ONLY of the bank is enabled or not
not everywhere.
>
> This patch is a sanity check.
You need only one at the entry point if you do have a NULL pointer accessed by the code
path the kernel need to oops so you known there is a problem, specially in secure mode.
This could means a bug or worse.
And This will cause CPU cycle consuming for nothing.
> It is a bit rude to nack this kind of
> patch telling we should never get this situation. There is always some
> cases we didn't think of. Moreover, I think this patch is in the view of
> the driver: the device can manage up to 160 pins but for some reasons, only
> 128 can be managed by Linux.
This an other issue.
>
>
>>
>> Best Regards,
>> J.
>>> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>>> mask = pin_to_mask(pin);
>>>
>>> --
>>> 2.4.4
>>>
>>
>
> Regards
>
> Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH] pinctrl: at91: fix null pointer dereference
2015-07-13 20:30 ` Jean-Christophe PLAGNIOL-VILLARD
@ 2015-07-15 13:30 ` Ludovic Desroches
-1 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-15 13:30 UTC (permalink / raw)
To: Jean-Christophe PLAGNIOL-VILLARD
Cc: Ludovic Desroches, David Dueck, Nicolas FERRE, Alexandre Belloni,
Boris Brezillon, linux-arm-kernel, linux-kernel
On Tue, Jul 14, 2015 at 04:30:14AM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> > On Jul 13, 2015, at 9:00 PM, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
> >
> > On Mon, Jul 13, 2015 at 02:14:51PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>
> >>> On Jul 3, 2015, at 12:06 AM, David Dueck <davidcdueck@googlemail.com> wrote:
> >>>
> >>> Not all gpio banks are necessarily enabled, in the current code this can
> >>> lead to null pointer dereferences.
> >>>
> >>> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> >>> [ 51.130000] pgd = dee04000
> >>> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> >>> [ 51.140000] Internal error: Oops: 17 [#1] ARM
> >>> [ 51.140000] Modules linked in:
> >>> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> >>> [ 51.140000] Hardware name: Atmel SAMA5
> >>> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> >>> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> >>> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> >>> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> >>> sp : dec61e48 ip : 600f0013 fp : df522538
> >>> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> >>> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> >>> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> >>> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> >>> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> >>> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> >>> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> >>> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> >>> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> >>> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> >>> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> >>> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> >>> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> >>> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> >>> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> >>> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> >>> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> >>> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> >>> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> >>> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> >>> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> >>> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> >>> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> >>> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> >>> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> >>> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> >>> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> >>> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> >>> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> >>> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
> >>>
> >>> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> >>> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> >>> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> >>> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> >>> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> >>> CC: linux-arm-kernel@lists.infradead.org
> >>> CC: linux-kernel@vger.kernel.org
> >>> ---
> >>> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> >>> 1 file changed, 15 insertions(+)
> >>>
> >>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> >>> index a082447..2deb130 100644
> >>> --- a/drivers/pinctrl/pinctrl-at91.c
> >>> +++ b/drivers/pinctrl/pinctrl-at91.c
> >>> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> >>> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> >>> unsigned int bank)
> >>> {
> >>> + if (!gpio_chips[bank])
> >>> + return NULL;
> >>
> >> if the bank is not enabled you should never arrived here
> >
> > But you can because in pinconf_pins_show you have:
> > for (i = 0; i < pctldev->desc->npins; i++)
> >
> > And in the pinctrl-at91 driver you have:
> > at91_pinctrl_desc.npins = gpio_banks * MAX_NB_GPIO_PER_BANK;
> >
> >>
> >>
> >>> +
> >>> return gpio_chips[bank]->regbase;
> >>> }
> >>>
> >>> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> >>> pin = &pins_conf[i];
> >>> at91_pin_dbg(info->dev, pin);
> >>> pio = pin_to_controller(info, pin->bank);
> >>> +
> >>> + if (!pio)
> >>> + continue;
> >>> +
> >>
> >> or here
> >>> mask = pin_to_mask(pin->pin);
> >>> at91_mux_disable_interrupt(pio, mask);
> >>> switch (pin->mux) {
> >>> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> >>> *config = 0;
> >>> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> >>> pio = pin_to_controller(info, pin_to_bank(pin_id));
> >>> +
> >>> + if (!pio)
> >>> + return -EINVAL;
> >>> +
> >> ditto
> >>> pin = pin_id % MAX_NB_GPIO_PER_BANK;
> >>>
> >>> if (at91_mux_get_multidrive(pio, pin))
> >>> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> >>> "%s:%d, pin_id=%d, config=0x%lx",
> >>> __func__, __LINE__, pin_id, config);
> >>> pio = pin_to_controller(info, pin_to_bank(pin_id));
> >>> +
> >>> + if (!pio)
> >>> + return -EINVAL;
> >>> +
> >>
> >> ditoo
> >>
> >> NACK
> >>
> >> the problem is somewhere else
> >
> > I agree that the root cause is somewhere else.
>
> So we MUST fix the root and not do quick fix
I agree there is a root cause leading to NULL pointers but why it should
be considered as a wrong behavior?
It is the patch you have proposed in order to allow having a disabled
pio controller which leads to have NULL pointers due to the way
gpio_chips is filled.
>
> > We have a
> > 'gpio_chips[MAX_GPIO_BANKS]' table and 'npins = gpio_banks *
> > MAX_NB_GPIO_PER_BANK' where 'gpio_banks = max(gpio_banks, alias_idx +
> > 1)'. If you have a disabled controller, you will have a NULL pointer in
> > the gpio_chips table and some pins which can't be accessed (for example
> > because this pio controller is secure only).
>
> I agree but you to catch in one point ONLY of the bank is enabled or not
> not everywhere.
>
> >
> > This patch is a sanity check.
>
> You need only one at the entry point if you do have a NULL pointer accessed by the code
> path the kernel need to oops so you known there is a problem, specially in secure mode.
> This could means a bug or worse.
You declare that you have 160 pins to Linux that's the SoC description so I
won't considered this as wrong.
You have a disabled pio controller for any reasons so some pins are not
accessible for Linux. It seems right too.
If you request one of these pins, the pin_to_controller() function will return
a NULL pointer then a EINVAL error will be returned. It is a proper way to
manage it. Why needing a oops in this case, an error is enough.
>
> And This will cause CPU cycle consuming for nothing.
>
> > It is a bit rude to nack this kind of
> > patch telling we should never get this situation. There is always some
> > cases we didn't think of. Moreover, I think this patch is in the view of
> > the driver: the device can manage up to 160 pins but for some reasons, only
> > 128 can be managed by Linux.
>
> This an other issue.
It is exactly the use case leading to this 'bug'.
Regards
Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-15 13:30 ` Ludovic Desroches
0 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-15 13:30 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 14, 2015 at 04:30:14AM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
>
> > On Jul 13, 2015, at 9:00 PM, Ludovic Desroches <ludovic.desroches@atmel.com> wrote:
> >
> > On Mon, Jul 13, 2015 at 02:14:51PM +0800, Jean-Christophe PLAGNIOL-VILLARD wrote:
> >>
> >>> On Jul 3, 2015, at 12:06 AM, David Dueck <davidcdueck@googlemail.com> wrote:
> >>>
> >>> Not all gpio banks are necessarily enabled, in the current code this can
> >>> lead to null pointer dereferences.
> >>>
> >>> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> >>> [ 51.130000] pgd = dee04000
> >>> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> >>> [ 51.140000] Internal error: Oops: 17 [#1] ARM
> >>> [ 51.140000] Modules linked in:
> >>> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> >>> [ 51.140000] Hardware name: Atmel SAMA5
> >>> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> >>> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> >>> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> >>> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> >>> sp : dec61e48 ip : 600f0013 fp : df522538
> >>> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> >>> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> >>> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> >>> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> >>> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> >>> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> >>> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> >>> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> >>> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> >>> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> >>> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> >>> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> >>> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> >>> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> >>> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> >>> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> >>> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> >>> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> >>> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> >>> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> >>> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> >>> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> >>> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> >>> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> >>> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> >>> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> >>> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> >>> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> >>> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> >>> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
> >>>
> >>> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> >>> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> >>> CC: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> >>> CC: Ludovic Desroches <ludovic.desroches@atmel.com>
> >>> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> >>> CC: linux-arm-kernel at lists.infradead.org
> >>> CC: linux-kernel at vger.kernel.org
> >>> ---
> >>> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> >>> 1 file changed, 15 insertions(+)
> >>>
> >>> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> >>> index a082447..2deb130 100644
> >>> --- a/drivers/pinctrl/pinctrl-at91.c
> >>> +++ b/drivers/pinctrl/pinctrl-at91.c
> >>> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> >>> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> >>> unsigned int bank)
> >>> {
> >>> + if (!gpio_chips[bank])
> >>> + return NULL;
> >>
> >> if the bank is not enabled you should never arrived here
> >
> > But you can because in pinconf_pins_show you have:
> > for (i = 0; i < pctldev->desc->npins; i++)
> >
> > And in the pinctrl-at91 driver you have:
> > at91_pinctrl_desc.npins = gpio_banks * MAX_NB_GPIO_PER_BANK;
> >
> >>
> >>
> >>> +
> >>> return gpio_chips[bank]->regbase;
> >>> }
> >>>
> >>> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> >>> pin = &pins_conf[i];
> >>> at91_pin_dbg(info->dev, pin);
> >>> pio = pin_to_controller(info, pin->bank);
> >>> +
> >>> + if (!pio)
> >>> + continue;
> >>> +
> >>
> >> or here
> >>> mask = pin_to_mask(pin->pin);
> >>> at91_mux_disable_interrupt(pio, mask);
> >>> switch (pin->mux) {
> >>> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> >>> *config = 0;
> >>> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> >>> pio = pin_to_controller(info, pin_to_bank(pin_id));
> >>> +
> >>> + if (!pio)
> >>> + return -EINVAL;
> >>> +
> >> ditto
> >>> pin = pin_id % MAX_NB_GPIO_PER_BANK;
> >>>
> >>> if (at91_mux_get_multidrive(pio, pin))
> >>> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> >>> "%s:%d, pin_id=%d, config=0x%lx",
> >>> __func__, __LINE__, pin_id, config);
> >>> pio = pin_to_controller(info, pin_to_bank(pin_id));
> >>> +
> >>> + if (!pio)
> >>> + return -EINVAL;
> >>> +
> >>
> >> ditoo
> >>
> >> NACK
> >>
> >> the problem is somewhere else
> >
> > I agree that the root cause is somewhere else.
>
> So we MUST fix the root and not do quick fix
I agree there is a root cause leading to NULL pointers but why it should
be considered as a wrong behavior?
It is the patch you have proposed in order to allow having a disabled
pio controller which leads to have NULL pointers due to the way
gpio_chips is filled.
>
> > We have a
> > 'gpio_chips[MAX_GPIO_BANKS]' table and 'npins = gpio_banks *
> > MAX_NB_GPIO_PER_BANK' where 'gpio_banks = max(gpio_banks, alias_idx +
> > 1)'. If you have a disabled controller, you will have a NULL pointer in
> > the gpio_chips table and some pins which can't be accessed (for example
> > because this pio controller is secure only).
>
> I agree but you to catch in one point ONLY of the bank is enabled or not
> not everywhere.
>
> >
> > This patch is a sanity check.
>
> You need only one at the entry point if you do have a NULL pointer accessed by the code
> path the kernel need to oops so you known there is a problem, specially in secure mode.
> This could means a bug or worse.
You declare that you have 160 pins to Linux that's the SoC description so I
won't considered this as wrong.
You have a disabled pio controller for any reasons so some pins are not
accessible for Linux. It seems right too.
If you request one of these pins, the pin_to_controller() function will return
a NULL pointer then a EINVAL error will be returned. It is a proper way to
manage it. Why needing a oops in this case, an error is enough.
>
> And This will cause CPU cycle consuming for nothing.
>
> > It is a bit rude to nack this kind of
> > patch telling we should never get this situation. There is always some
> > cases we didn't think of. Moreover, I think this patch is in the view of
> > the driver: the device can manage up to 160 pins but for some reasons, only
> > 128 can be managed by Linux.
>
> This an other issue.
It is exactly the use case leading to this 'bug'.
Regards
Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RESEND PATCH] pinctrl: at91: fix null pointer dereference
2015-07-15 13:30 ` Ludovic Desroches
(?)
@ 2015-07-28 7:48 ` Ludovic Desroches
-1 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-28 7:48 UTC (permalink / raw)
To: linux-gpio
Cc: plagnioj, davidcdueck, alexandre.belloni, Nicolas Ferre,
Boris Brezillon, linux-arm-kernel, linux-kernel
From: David Dueck <davidcdueck@googlemail.com>
Not all gpio banks are necessarily enabled, in the current code this can
lead to null pointer dereferences.
[ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
[ 51.130000] pgd = dee04000
[ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
[ 51.140000] Internal error: Oops: 17 [#1] ARM
[ 51.140000] Modules linked in:
[ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
[ 51.140000] Hardware name: Atmel SAMA5
[ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
[ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
[ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
[ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
sp : dec61e48 ip : 600f0013 fp : df522538
[ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
[ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
[ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
[ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
[ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
[ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
[ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
[ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
[ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
[ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
[ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
[ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
[ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
[ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
[ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
[ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
[ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
[ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
[ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
[ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
[ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
[ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
[ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
[ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
[ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
[ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
[ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
[ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
[ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
Signed-off-by: David Dueck <davidcdueck@googlemail.com>
Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
CC: Nicolas Ferre <nicolas.ferre@atmel.com>
CC: Boris Brezillon <boris.brezillon@free-electrons.com>
CC: linux-arm-kernel@lists.infradead.org
CC: linux-kernel@vger.kernel.org
---
This patch fixes a oops in the kernel because of a NULL pointer in a table.
Having a NULL pointer in this table is the normal behavior if a PIO controller
is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
an entry from a table if it is not filled.
drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index a082447..2deb130 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
static void __iomem *pin_to_controller(struct at91_pinctrl *info,
unsigned int bank)
{
+ if (!gpio_chips[bank])
+ return NULL;
+
return gpio_chips[bank]->regbase;
}
@@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
pin = &pins_conf[i];
at91_pin_dbg(info->dev, pin);
pio = pin_to_controller(info, pin->bank);
+
+ if (!pio)
+ continue;
+
mask = pin_to_mask(pin->pin);
at91_mux_disable_interrupt(pio, mask);
switch (pin->mux) {
@@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
*config = 0;
dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+ if (!pio)
+ return -EINVAL;
+
pin = pin_id % MAX_NB_GPIO_PER_BANK;
if (at91_mux_get_multidrive(pio, pin))
@@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
"%s:%d, pin_id=%d, config=0x%lx",
__func__, __LINE__, pin_id, config);
pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+ if (!pio)
+ return -EINVAL;
+
pin = pin_id % MAX_NB_GPIO_PER_BANK;
mask = pin_to_mask(pin);
--
2.2.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-28 7:48 ` Ludovic Desroches
0 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-28 7:48 UTC (permalink / raw)
To: linux-gpio
Cc: plagnioj, davidcdueck, alexandre.belloni, Nicolas Ferre,
Boris Brezillon, linux-arm-kernel, linux-kernel
From: David Dueck <davidcdueck@googlemail.com>
Not all gpio banks are necessarily enabled, in the current code this can
lead to null pointer dereferences.
[ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
[ 51.130000] pgd = dee04000
[ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
[ 51.140000] Internal error: Oops: 17 [#1] ARM
[ 51.140000] Modules linked in:
[ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
[ 51.140000] Hardware name: Atmel SAMA5
[ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
[ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
[ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
[ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
sp : dec61e48 ip : 600f0013 fp : df522538
[ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
[ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
[ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
[ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
[ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
[ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
[ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
[ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
[ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
[ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
[ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
[ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
[ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
[ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
[ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
[ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
[ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
[ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
[ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
[ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
[ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
[ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
[ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
[ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
[ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
[ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
[ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
[ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
[ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
Signed-off-by: David Dueck <davidcdueck@googlemail.com>
Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
CC: Nicolas Ferre <nicolas.ferre@atmel.com>
CC: Boris Brezillon <boris.brezillon@free-electrons.com>
CC: linux-arm-kernel@lists.infradead.org
CC: linux-kernel@vger.kernel.org
---
This patch fixes a oops in the kernel because of a NULL pointer in a table.
Having a NULL pointer in this table is the normal behavior if a PIO controller
is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
an entry from a table if it is not filled.
drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index a082447..2deb130 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
static void __iomem *pin_to_controller(struct at91_pinctrl *info,
unsigned int bank)
{
+ if (!gpio_chips[bank])
+ return NULL;
+
return gpio_chips[bank]->regbase;
}
@@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
pin = &pins_conf[i];
at91_pin_dbg(info->dev, pin);
pio = pin_to_controller(info, pin->bank);
+
+ if (!pio)
+ continue;
+
mask = pin_to_mask(pin->pin);
at91_mux_disable_interrupt(pio, mask);
switch (pin->mux) {
@@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
*config = 0;
dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+ if (!pio)
+ return -EINVAL;
+
pin = pin_id % MAX_NB_GPIO_PER_BANK;
if (at91_mux_get_multidrive(pio, pin))
@@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
"%s:%d, pin_id=%d, config=0x%lx",
__func__, __LINE__, pin_id, config);
pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+ if (!pio)
+ return -EINVAL;
+
pin = pin_id % MAX_NB_GPIO_PER_BANK;
mask = pin_to_mask(pin);
--
2.2.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-28 7:48 ` Ludovic Desroches
0 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-28 7:48 UTC (permalink / raw)
To: linux-arm-kernel
From: David Dueck <davidcdueck@googlemail.com>
Not all gpio banks are necessarily enabled, in the current code this can
lead to null pointer dereferences.
[ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
[ 51.130000] pgd = dee04000
[ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
[ 51.140000] Internal error: Oops: 17 [#1] ARM
[ 51.140000] Modules linked in:
[ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
[ 51.140000] Hardware name: Atmel SAMA5
[ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
[ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
[ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
[ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
sp : dec61e48 ip : 600f0013 fp : df522538
[ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
[ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
[ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
[ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
[ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
[ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
[ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
[ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
[ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
[ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
[ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
[ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
[ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
[ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
[ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
[ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
[ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
[ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
[ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
[ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
[ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
[ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
[ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
[ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
[ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
[ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
[ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
[ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
[ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
Signed-off-by: David Dueck <davidcdueck@googlemail.com>
Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
CC: Nicolas Ferre <nicolas.ferre@atmel.com>
CC: Boris Brezillon <boris.brezillon@free-electrons.com>
CC: linux-arm-kernel at lists.infradead.org
CC: linux-kernel at vger.kernel.org
---
This patch fixes a oops in the kernel because of a NULL pointer in a table.
Having a NULL pointer in this table is the normal behavior if a PIO controller
is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
an entry from a table if it is not filled.
drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
index a082447..2deb130 100644
--- a/drivers/pinctrl/pinctrl-at91.c
+++ b/drivers/pinctrl/pinctrl-at91.c
@@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
static void __iomem *pin_to_controller(struct at91_pinctrl *info,
unsigned int bank)
{
+ if (!gpio_chips[bank])
+ return NULL;
+
return gpio_chips[bank]->regbase;
}
@@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
pin = &pins_conf[i];
at91_pin_dbg(info->dev, pin);
pio = pin_to_controller(info, pin->bank);
+
+ if (!pio)
+ continue;
+
mask = pin_to_mask(pin->pin);
at91_mux_disable_interrupt(pio, mask);
switch (pin->mux) {
@@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
*config = 0;
dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+ if (!pio)
+ return -EINVAL;
+
pin = pin_id % MAX_NB_GPIO_PER_BANK;
if (at91_mux_get_multidrive(pio, pin))
@@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
"%s:%d, pin_id=%d, config=0x%lx",
__func__, __LINE__, pin_id, config);
pio = pin_to_controller(info, pin_to_bank(pin_id));
+
+ if (!pio)
+ return -EINVAL;
+
pin = pin_id % MAX_NB_GPIO_PER_BANK;
mask = pin_to_mask(pin);
--
2.2.0
^ permalink raw reply related [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
2015-07-28 7:48 ` Ludovic Desroches
(?)
@ 2015-07-28 8:00 ` Nicolas Ferre
-1 siblings, 0 replies; 39+ messages in thread
From: Nicolas Ferre @ 2015-07-28 8:00 UTC (permalink / raw)
To: Ludovic Desroches, linux-gpio, Linus Walleij
Cc: plagnioj, davidcdueck, alexandre.belloni, Boris Brezillon,
linux-arm-kernel, linux-kernel
Le 28/07/2015 09:48, Ludovic Desroches a écrit :
> From: David Dueck <davidcdueck@googlemail.com>
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> [ 51.130000] pgd = dee04000
> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> [ 51.140000] Internal error: Oops: 17 [#1] ARM
> [ 51.140000] Modules linked in:
> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> [ 51.140000] Hardware name: Atmel SAMA5
> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> sp : dec61e48 ip : 600f0013 fp : df522538
> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> ---
>
> This patch fixes a oops in the kernel because of a NULL pointer in a table.
> Having a NULL pointer in this table is the normal behavior if a PIO controller
> is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> an entry from a table if it is not filled.
Absolutely.
Thanks David for having written the fix and Ludovic for having verified
and confirmed that it's actually the proper way to fix the bug.
Bye,
> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a082447..2deb130 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> unsigned int bank)
> {
> + if (!gpio_chips[bank])
> + return NULL;
> +
> return gpio_chips[bank]->regbase;
> }
>
> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> pin = &pins_conf[i];
> at91_pin_dbg(info->dev, pin);
> pio = pin_to_controller(info, pin->bank);
> +
> + if (!pio)
> + continue;
> +
> mask = pin_to_mask(pin->pin);
> at91_mux_disable_interrupt(pio, mask);
> switch (pin->mux) {
> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> *config = 0;
> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
> if (at91_mux_get_multidrive(pio, pin))
> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> "%s:%d, pin_id=%d, config=0x%lx",
> __func__, __LINE__, pin_id, config);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
> mask = pin_to_mask(pin);
>
>
--
Nicolas Ferre
--
To unsubscribe from this list: send the line "unsubscribe linux-gpio" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-28 8:00 ` Nicolas Ferre
0 siblings, 0 replies; 39+ messages in thread
From: Nicolas Ferre @ 2015-07-28 8:00 UTC (permalink / raw)
To: Ludovic Desroches, linux-gpio, Linus Walleij
Cc: plagnioj, davidcdueck, alexandre.belloni, Boris Brezillon,
linux-arm-kernel, linux-kernel
Le 28/07/2015 09:48, Ludovic Desroches a écrit :
> From: David Dueck <davidcdueck@googlemail.com>
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> [ 51.130000] pgd = dee04000
> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> [ 51.140000] Internal error: Oops: 17 [#1] ARM
> [ 51.140000] Modules linked in:
> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> [ 51.140000] Hardware name: Atmel SAMA5
> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> sp : dec61e48 ip : 600f0013 fp : df522538
> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> ---
>
> This patch fixes a oops in the kernel because of a NULL pointer in a table.
> Having a NULL pointer in this table is the normal behavior if a PIO controller
> is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> an entry from a table if it is not filled.
Absolutely.
Thanks David for having written the fix and Ludovic for having verified
and confirmed that it's actually the proper way to fix the bug.
Bye,
> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a082447..2deb130 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> unsigned int bank)
> {
> + if (!gpio_chips[bank])
> + return NULL;
> +
> return gpio_chips[bank]->regbase;
> }
>
> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> pin = &pins_conf[i];
> at91_pin_dbg(info->dev, pin);
> pio = pin_to_controller(info, pin->bank);
> +
> + if (!pio)
> + continue;
> +
> mask = pin_to_mask(pin->pin);
> at91_mux_disable_interrupt(pio, mask);
> switch (pin->mux) {
> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> *config = 0;
> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
> if (at91_mux_get_multidrive(pio, pin))
> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> "%s:%d, pin_id=%d, config=0x%lx",
> __func__, __LINE__, pin_id, config);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
> mask = pin_to_mask(pin);
>
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-28 8:00 ` Nicolas Ferre
0 siblings, 0 replies; 39+ messages in thread
From: Nicolas Ferre @ 2015-07-28 8:00 UTC (permalink / raw)
To: linux-arm-kernel
Le 28/07/2015 09:48, Ludovic Desroches a ?crit :
> From: David Dueck <davidcdueck@googlemail.com>
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> [ 51.130000] pgd = dee04000
> [ 51.130000] [00000058] *pgd=3f66d831, *pte=00000000, *ppte=00000000
> [ 51.140000] Internal error: Oops: 17 [#1] ARM
> [ 51.140000] Modules linked in:
> [ 51.140000] CPU: 0 PID: 1664 Comm: cat Not tainted 4.1.1+ #6
> [ 51.140000] Hardware name: Atmel SAMA5
> [ 51.140000] task: df6dd880 ti: dec60000 task.ti: dec60000
> [ 51.140000] PC is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] LR is at at91_pinconf_get+0xb4/0x200
> [ 51.140000] pc : [<c01e71a0>] lr : [<c01e71a0>] psr: 600f0013
> sp : dec61e48 ip : 600f0013 fp : df522538
> [ 51.140000] r10: df52250c r9 : 00000058 r8 : 00000068
> [ 51.140000] r7 : 00000000 r6 : df53c910 r5 : 00000000 r4 : dec61e7c
> [ 51.140000] r3 : 00000000 r2 : c06746d4 r1 : 00000000 r0 : 00000003
> [ 51.140000] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [ 51.140000] Control: 10c53c7d Table: 3ee04059 DAC: 00000015
> [ 51.140000] Process cat (pid: 1664, stack limit = 0xdec60208)
> [ 51.140000] Stack: (0xdec61e48 to 0xdec62000)
> [ 51.140000] 1e40: 00000358 00000000 df522500 ded15f80 c05a9d08 ded15f80
> [ 51.140000] 1e60: 0000048c 00000061 df522500 ded15f80 c05a9d08 c01e7304 ded15f80 00000000
> [ 51.140000] 1e80: c01e6008 00000060 0000048c c01e6034 c01e5f6c ded15f80 dec61ec0 00000000
> [ 51.140000] 1ea0: 00020000 ded6f280 dec61f80 00000001 00000001 c00ae0b8 b6e80000 ded15fb0
> [ 51.140000] 1ec0: 00000000 00000000 df4bc974 00000055 00000800 ded6f280 b6e80000 ded6f280
> [ 51.140000] 1ee0: ded6f280 00020000 b6e80000 00000000 00020000 c0090dec c0671e1c dec61fb0
> [ 51.140000] 1f00: b6f8b510 00000001 00004201 c000924c 00000000 00000003 00000003 00000000
> [ 51.140000] 1f20: df4bc940 00022000 00000022 c066e188 b6e7f000 c00836f4 000b6e7f ded6f280
> [ 51.140000] 1f40: ded6f280 b6e80000 dec61f80 ded6f280 00020000 c0091508 00000000 00000003
> [ 51.140000] 1f60: 00022000 00000000 00000000 ded6f280 ded6f280 00020000 b6e80000 c0091d9c
> [ 51.140000] 1f80: 00000000 00000000 ffffffff 00020000 00020000 b6e80000 00000003 c000f124
> [ 51.140000] 1fa0: dec60000 c000efa0 00020000 00020000 00000003 b6e80000 00020000 000271c4
> [ 51.140000] 1fc0: 00020000 00020000 b6e80000 00000003 7fffe000 00000000 00000000 00020000
> [ 51.140000] 1fe0: 00000000 bef50b64 00013835 b6f29c76 400f0030 00000003 00000000 00000000
> [ 51.140000] [<c01e71a0>] (at91_pinconf_get) from [<c01e7304>] (at91_pinconf_dbg_show+0x18/0x2c0)
> [ 51.140000] [<c01e7304>] (at91_pinconf_dbg_show) from [<c01e6034>] (pinconf_pins_show+0xc8/0xf8)
> [ 51.140000] [<c01e6034>] (pinconf_pins_show) from [<c00ae0b8>] (seq_read+0x1a0/0x464)
> [ 51.140000] [<c00ae0b8>] (seq_read) from [<c0090dec>] (__vfs_read+0x20/0xd0)
> [ 51.140000] [<c0090dec>] (__vfs_read) from [<c0091508>] (vfs_read+0x7c/0x108)
> [ 51.140000] [<c0091508>] (vfs_read) from [<c0091d9c>] (SyS_read+0x40/0x94)
> [ 51.140000] [<c0091d9c>] (SyS_read) from [<c000efa0>] (ret_fast_syscall+0x0/0x3c)
> [ 51.140000] Code: eb010ec2 e30a0d08 e34c005a eb0ae5a7 (e5993000)
> [ 51.150000] ---[ end trace fb3c370da3ea4794 ]---
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-kernel at vger.kernel.org
> ---
>
> This patch fixes a oops in the kernel because of a NULL pointer in a table.
> Having a NULL pointer in this table is the normal behavior if a PIO controller
> is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> an entry from a table if it is not filled.
Absolutely.
Thanks David for having written the fix and Ludovic for having verified
and confirmed that it's actually the proper way to fix the bug.
Bye,
> drivers/pinctrl/pinctrl-at91.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pinctrl/pinctrl-at91.c b/drivers/pinctrl/pinctrl-at91.c
> index a082447..2deb130 100644
> --- a/drivers/pinctrl/pinctrl-at91.c
> +++ b/drivers/pinctrl/pinctrl-at91.c
> @@ -320,6 +320,9 @@ static const struct pinctrl_ops at91_pctrl_ops = {
> static void __iomem *pin_to_controller(struct at91_pinctrl *info,
> unsigned int bank)
> {
> + if (!gpio_chips[bank])
> + return NULL;
> +
> return gpio_chips[bank]->regbase;
> }
>
> @@ -729,6 +732,10 @@ static int at91_pmx_set(struct pinctrl_dev *pctldev, unsigned selector,
> pin = &pins_conf[i];
> at91_pin_dbg(info->dev, pin);
> pio = pin_to_controller(info, pin->bank);
> +
> + if (!pio)
> + continue;
> +
> mask = pin_to_mask(pin->pin);
> at91_mux_disable_interrupt(pio, mask);
> switch (pin->mux) {
> @@ -848,6 +855,10 @@ static int at91_pinconf_get(struct pinctrl_dev *pctldev,
> *config = 0;
> dev_dbg(info->dev, "%s:%d, pin_id=%d", __func__, __LINE__, pin_id);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
>
> if (at91_mux_get_multidrive(pio, pin))
> @@ -889,6 +900,10 @@ static int at91_pinconf_set(struct pinctrl_dev *pctldev,
> "%s:%d, pin_id=%d, config=0x%lx",
> __func__, __LINE__, pin_id, config);
> pio = pin_to_controller(info, pin_to_bank(pin_id));
> +
> + if (!pio)
> + return -EINVAL;
> +
> pin = pin_id % MAX_NB_GPIO_PER_BANK;
> mask = pin_to_mask(pin);
>
>
--
Nicolas Ferre
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
2015-07-28 7:48 ` Ludovic Desroches
(?)
@ 2015-07-28 12:48 ` Linus Walleij
-1 siblings, 0 replies; 39+ messages in thread
From: Linus Walleij @ 2015-07-28 12:48 UTC (permalink / raw)
To: Ludovic Desroches
Cc: linux-gpio, Jean-Christophe PLAGNIOL-VILLARD, davidcdueck,
Alexandre Belloni, Nicolas Ferre, Boris Brezillon,
linux-arm-kernel, linux-kernel
On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
<ludovic.desroches@atmel.com> wrote:
> From: David Dueck <davidcdueck@googlemail.com>
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
(...)
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> ---
>
> This patch fixes a oops in the kernel because of a NULL pointer in a table.
> Having a NULL pointer in this table is the normal behavior if a PIO controller
> is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> an entry from a table if it is not filled.
Fair enough, better too many checks than too few.
Is this a regression to v4.2 that should go to stable or v4.3 material?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-28 12:48 ` Linus Walleij
0 siblings, 0 replies; 39+ messages in thread
From: Linus Walleij @ 2015-07-28 12:48 UTC (permalink / raw)
To: Ludovic Desroches
Cc: linux-gpio, Jean-Christophe PLAGNIOL-VILLARD, davidcdueck,
Alexandre Belloni, Nicolas Ferre, Boris Brezillon,
linux-arm-kernel, linux-kernel
On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
<ludovic.desroches@atmel.com> wrote:
> From: David Dueck <davidcdueck@googlemail.com>
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
(...)
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> ---
>
> This patch fixes a oops in the kernel because of a NULL pointer in a table.
> Having a NULL pointer in this table is the normal behavior if a PIO controller
> is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> an entry from a table if it is not filled.
Fair enough, better too many checks than too few.
Is this a regression to v4.2 that should go to stable or v4.3 material?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-28 12:48 ` Linus Walleij
0 siblings, 0 replies; 39+ messages in thread
From: Linus Walleij @ 2015-07-28 12:48 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
<ludovic.desroches@atmel.com> wrote:
> From: David Dueck <davidcdueck@googlemail.com>
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
>
> [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
(...)
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-kernel at vger.kernel.org
> ---
>
> This patch fixes a oops in the kernel because of a NULL pointer in a table.
> Having a NULL pointer in this table is the normal behavior if a PIO controller
> is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> an entry from a table if it is not filled.
Fair enough, better too many checks than too few.
Is this a regression to v4.2 that should go to stable or v4.3 material?
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
2015-07-28 12:48 ` Linus Walleij
(?)
@ 2015-07-28 13:12 ` Ludovic Desroches
-1 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-28 13:12 UTC (permalink / raw)
To: Linus Walleij
Cc: Ludovic Desroches, linux-gpio, Jean-Christophe PLAGNIOL-VILLARD,
davidcdueck, Alexandre Belloni, Nicolas Ferre, Boris Brezillon,
linux-arm-kernel, linux-kernel
On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
> <ludovic.desroches@atmel.com> wrote:
>
> > From: David Dueck <davidcdueck@googlemail.com>
> >
> > Not all gpio banks are necessarily enabled, in the current code this can
> > lead to null pointer dereferences.
> >
> > [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> (...)
> >
> > Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> > Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> > CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> > CC: linux-arm-kernel@lists.infradead.org
> > CC: linux-kernel@vger.kernel.org
> > ---
> >
> > This patch fixes a oops in the kernel because of a NULL pointer in a table.
> > Having a NULL pointer in this table is the normal behavior if a PIO controller
> > is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> > an entry from a table if it is not filled.
>
> Fair enough, better too many checks than too few.
>
> Is this a regression to v4.2 that should go to stable or v4.3 material?
Yes it is a regression from v4.0, it applies well on v4.0.9
Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
Cc: stable@vger.kernel.org # 4.0
Thanks
Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-28 13:12 ` Ludovic Desroches
0 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-28 13:12 UTC (permalink / raw)
To: Linus Walleij
Cc: Ludovic Desroches, linux-gpio, Jean-Christophe PLAGNIOL-VILLARD,
davidcdueck, Alexandre Belloni, Nicolas Ferre, Boris Brezillon,
linux-arm-kernel, linux-kernel
On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
> <ludovic.desroches@atmel.com> wrote:
>
> > From: David Dueck <davidcdueck@googlemail.com>
> >
> > Not all gpio banks are necessarily enabled, in the current code this can
> > lead to null pointer dereferences.
> >
> > [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> (...)
> >
> > Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> > Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> > CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> > CC: linux-arm-kernel@lists.infradead.org
> > CC: linux-kernel@vger.kernel.org
> > ---
> >
> > This patch fixes a oops in the kernel because of a NULL pointer in a table.
> > Having a NULL pointer in this table is the normal behavior if a PIO controller
> > is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> > an entry from a table if it is not filled.
>
> Fair enough, better too many checks than too few.
>
> Is this a regression to v4.2 that should go to stable or v4.3 material?
Yes it is a regression from v4.0, it applies well on v4.0.9
Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
Cc: stable@vger.kernel.org # 4.0
Thanks
Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-28 13:12 ` Ludovic Desroches
0 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-28 13:12 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
> <ludovic.desroches@atmel.com> wrote:
>
> > From: David Dueck <davidcdueck@googlemail.com>
> >
> > Not all gpio banks are necessarily enabled, in the current code this can
> > lead to null pointer dereferences.
> >
> > [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> (...)
> >
> > Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> > Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> > CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> > CC: linux-arm-kernel at lists.infradead.org
> > CC: linux-kernel at vger.kernel.org
> > ---
> >
> > This patch fixes a oops in the kernel because of a NULL pointer in a table.
> > Having a NULL pointer in this table is the normal behavior if a PIO controller
> > is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> > an entry from a table if it is not filled.
>
> Fair enough, better too many checks than too few.
>
> Is this a regression to v4.2 that should go to stable or v4.3 material?
Yes it is a regression from v4.0, it applies well on v4.0.9
Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
Cc: stable at vger.kernel.org # 4.0
Thanks
Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
2015-07-28 13:12 ` Ludovic Desroches
(?)
@ 2015-07-30 8:33 ` Sylvain Rochet
-1 siblings, 0 replies; 39+ messages in thread
From: Sylvain Rochet @ 2015-07-30 8:33 UTC (permalink / raw)
To: Ludovic Desroches
Cc: Linus Walleij, linux-gpio, Jean-Christophe PLAGNIOL-VILLARD,
davidcdueck, Alexandre Belloni, Nicolas Ferre, Boris Brezillon,
linux-arm-kernel, linux-kernel
Hello Ludovic,
On Tue, Jul 28, 2015 at 03:12:53PM +0200, Ludovic Desroches wrote:
> On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> > On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
> > <ludovic.desroches@atmel.com> wrote:
> >
> > > From: David Dueck <davidcdueck@googlemail.com>
> > >
> > > Not all gpio banks are necessarily enabled, in the current code this can
> > > lead to null pointer dereferences.
> > >
> > > [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> > (...)
> > >
> > > Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> > > Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > > Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > > CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> > > CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > CC: linux-arm-kernel@lists.infradead.org
> > > CC: linux-kernel@vger.kernel.org
> > > ---
> > >
> > > This patch fixes a oops in the kernel because of a NULL pointer in a table.
> > > Having a NULL pointer in this table is the normal behavior if a PIO controller
> > > is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> > > an entry from a table if it is not filled.
> >
> > Fair enough, better too many checks than too few.
> >
> > Is this a regression to v4.2 that should go to stable or v4.3 material?
>
> Yes it is a regression from v4.0, it applies well on v4.0.9
>
> Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
> Cc: stable@vger.kernel.org # 4.0
a0b957f306fa have a stable tag up to 3.18, should this patch inherit a
stable tag up to 3.18 instead of only up to 4.0 ?
Sylvain
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-30 8:33 ` Sylvain Rochet
0 siblings, 0 replies; 39+ messages in thread
From: Sylvain Rochet @ 2015-07-30 8:33 UTC (permalink / raw)
To: Ludovic Desroches
Cc: Linus Walleij, linux-gpio, Jean-Christophe PLAGNIOL-VILLARD,
davidcdueck, Alexandre Belloni, Nicolas Ferre, Boris Brezillon,
linux-arm-kernel, linux-kernel
Hello Ludovic,
On Tue, Jul 28, 2015 at 03:12:53PM +0200, Ludovic Desroches wrote:
> On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> > On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
> > <ludovic.desroches@atmel.com> wrote:
> >
> > > From: David Dueck <davidcdueck@googlemail.com>
> > >
> > > Not all gpio banks are necessarily enabled, in the current code this can
> > > lead to null pointer dereferences.
> > >
> > > [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> > (...)
> > >
> > > Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> > > Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > > Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > > CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> > > CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > CC: linux-arm-kernel@lists.infradead.org
> > > CC: linux-kernel@vger.kernel.org
> > > ---
> > >
> > > This patch fixes a oops in the kernel because of a NULL pointer in a table.
> > > Having a NULL pointer in this table is the normal behavior if a PIO controller
> > > is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> > > an entry from a table if it is not filled.
> >
> > Fair enough, better too many checks than too few.
> >
> > Is this a regression to v4.2 that should go to stable or v4.3 material?
>
> Yes it is a regression from v4.0, it applies well on v4.0.9
>
> Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
> Cc: stable@vger.kernel.org # 4.0
a0b957f306fa have a stable tag up to 3.18, should this patch inherit a
stable tag up to 3.18 instead of only up to 4.0 ?
Sylvain
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-30 8:33 ` Sylvain Rochet
0 siblings, 0 replies; 39+ messages in thread
From: Sylvain Rochet @ 2015-07-30 8:33 UTC (permalink / raw)
To: linux-arm-kernel
Hello Ludovic,
On Tue, Jul 28, 2015 at 03:12:53PM +0200, Ludovic Desroches wrote:
> On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> > On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
> > <ludovic.desroches@atmel.com> wrote:
> >
> > > From: David Dueck <davidcdueck@googlemail.com>
> > >
> > > Not all gpio banks are necessarily enabled, in the current code this can
> > > lead to null pointer dereferences.
> > >
> > > [ 51.130000] Unable to handle kernel NULL pointer dereference at virtual address 00000058
> > (...)
> > >
> > > Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> > > Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> > > Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> > > CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> > > CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > CC: linux-arm-kernel at lists.infradead.org
> > > CC: linux-kernel at vger.kernel.org
> > > ---
> > >
> > > This patch fixes a oops in the kernel because of a NULL pointer in a table.
> > > Having a NULL pointer in this table is the normal behavior if a PIO controller
> > > is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> > > an entry from a table if it is not filled.
> >
> > Fair enough, better too many checks than too few.
> >
> > Is this a regression to v4.2 that should go to stable or v4.3 material?
>
> Yes it is a regression from v4.0, it applies well on v4.0.9
>
> Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
> Cc: stable at vger.kernel.org # 4.0
a0b957f306fa have a stable tag up to 3.18, should this patch inherit a
stable tag up to 3.18 instead of only up to 4.0 ?
Sylvain
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
2015-07-30 8:33 ` Sylvain Rochet
(?)
@ 2015-07-30 8:49 ` Ludovic Desroches
-1 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-30 8:49 UTC (permalink / raw)
To: Sylvain Rochet
Cc: Ludovic Desroches, Linus Walleij, linux-gpio,
Jean-Christophe PLAGNIOL-VILLARD, davidcdueck, Alexandre Belloni,
Nicolas Ferre, Boris Brezillon, linux-arm-kernel, linux-kernel
On Thu, Jul 30, 2015 at 10:33:43AM +0200, Sylvain Rochet wrote:
> Hello Ludovic,
>
> On Tue, Jul 28, 2015 at 03:12:53PM +0200, Ludovic Desroches wrote:
> > On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> > > On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
[...]
> > > Is this a regression to v4.2 that should go to stable or v4.3 material?
> >
> > Yes it is a regression from v4.0, it applies well on v4.0.9
> >
> > Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
> > Cc: stable@vger.kernel.org # 4.0
>
> a0b957f306fa have a stable tag up to 3.18, should this patch inherit a
> stable tag up to 3.18 instead of only up to 4.0 ?
>
Well spotted, I didn't notice it has hit 3.18 through stable, so yes
stable tag should be 3.18 and later.
Thanks
Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-30 8:49 ` Ludovic Desroches
0 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-30 8:49 UTC (permalink / raw)
To: Sylvain Rochet
Cc: Ludovic Desroches, Linus Walleij, linux-gpio,
Jean-Christophe PLAGNIOL-VILLARD, davidcdueck, Alexandre Belloni,
Nicolas Ferre, Boris Brezillon, linux-arm-kernel, linux-kernel
On Thu, Jul 30, 2015 at 10:33:43AM +0200, Sylvain Rochet wrote:
> Hello Ludovic,
>
> On Tue, Jul 28, 2015 at 03:12:53PM +0200, Ludovic Desroches wrote:
> > On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> > > On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
[...]
> > > Is this a regression to v4.2 that should go to stable or v4.3 material?
> >
> > Yes it is a regression from v4.0, it applies well on v4.0.9
> >
> > Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
> > Cc: stable@vger.kernel.org # 4.0
>
> a0b957f306fa have a stable tag up to 3.18, should this patch inherit a
> stable tag up to 3.18 instead of only up to 4.0 ?
>
Well spotted, I didn't notice it has hit 3.18 through stable, so yes
stable tag should be 3.18 and later.
Thanks
Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-30 8:49 ` Ludovic Desroches
0 siblings, 0 replies; 39+ messages in thread
From: Ludovic Desroches @ 2015-07-30 8:49 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Jul 30, 2015 at 10:33:43AM +0200, Sylvain Rochet wrote:
> Hello Ludovic,
>
> On Tue, Jul 28, 2015 at 03:12:53PM +0200, Ludovic Desroches wrote:
> > On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> > > On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
[...]
> > > Is this a regression to v4.2 that should go to stable or v4.3 material?
> >
> > Yes it is a regression from v4.0, it applies well on v4.0.9
> >
> > Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
> > Cc: stable at vger.kernel.org # 4.0
>
> a0b957f306fa have a stable tag up to 3.18, should this patch inherit a
> stable tag up to 3.18 instead of only up to 4.0 ?
>
Well spotted, I didn't notice it has hit 3.18 through stable, so yes
stable tag should be 3.18 and later.
Thanks
Ludovic
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
2015-07-30 8:49 ` Ludovic Desroches
(?)
@ 2015-07-30 9:16 ` Sylvain Rochet
-1 siblings, 0 replies; 39+ messages in thread
From: Sylvain Rochet @ 2015-07-30 9:16 UTC (permalink / raw)
To: Ludovic Desroches, Linus Walleij
Cc: linux-gpio, Jean-Christophe PLAGNIOL-VILLARD, davidcdueck,
Alexandre Belloni, Nicolas Ferre, Boris Brezillon,
linux-arm-kernel, linux-kernel
Hello,
On Thu, Jul 30, 2015 at 10:49:27AM +0200, Ludovic Desroches wrote:
> On Thu, Jul 30, 2015 at 10:33:43AM +0200, Sylvain Rochet wrote:
> > Hello Ludovic,
> >
> > On Tue, Jul 28, 2015 at 03:12:53PM +0200, Ludovic Desroches wrote:
> > > On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> > > > On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
>
> [...]
>
> > > > Is this a regression to v4.2 that should go to stable or v4.3 material?
> > >
> > > Yes it is a regression from v4.0, it applies well on v4.0.9
> > >
> > > Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
> > > Cc: stable@vger.kernel.org # 4.0
> >
> > a0b957f306fa have a stable tag up to 3.18, should this patch inherit a
> > stable tag up to 3.18 instead of only up to 4.0 ?
>
> Well spotted, I didn't notice it has hit 3.18 through stable, so yes
> stable tag should be 3.18 and later.
Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
Cc: stable@vger.kernel.org # 3.18
Sylvain
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-30 9:16 ` Sylvain Rochet
0 siblings, 0 replies; 39+ messages in thread
From: Sylvain Rochet @ 2015-07-30 9:16 UTC (permalink / raw)
To: Ludovic Desroches, Linus Walleij
Cc: linux-gpio, Jean-Christophe PLAGNIOL-VILLARD, davidcdueck,
Alexandre Belloni, Nicolas Ferre, Boris Brezillon,
linux-arm-kernel, linux-kernel
Hello,
On Thu, Jul 30, 2015 at 10:49:27AM +0200, Ludovic Desroches wrote:
> On Thu, Jul 30, 2015 at 10:33:43AM +0200, Sylvain Rochet wrote:
> > Hello Ludovic,
> >
> > On Tue, Jul 28, 2015 at 03:12:53PM +0200, Ludovic Desroches wrote:
> > > On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> > > > On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
>
> [...]
>
> > > > Is this a regression to v4.2 that should go to stable or v4.3 material?
> > >
> > > Yes it is a regression from v4.0, it applies well on v4.0.9
> > >
> > > Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
> > > Cc: stable@vger.kernel.org # 4.0
> >
> > a0b957f306fa have a stable tag up to 3.18, should this patch inherit a
> > stable tag up to 3.18 instead of only up to 4.0 ?
>
> Well spotted, I didn't notice it has hit 3.18 through stable, so yes
> stable tag should be 3.18 and later.
Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
Cc: stable@vger.kernel.org # 3.18
Sylvain
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-07-30 9:16 ` Sylvain Rochet
0 siblings, 0 replies; 39+ messages in thread
From: Sylvain Rochet @ 2015-07-30 9:16 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Thu, Jul 30, 2015 at 10:49:27AM +0200, Ludovic Desroches wrote:
> On Thu, Jul 30, 2015 at 10:33:43AM +0200, Sylvain Rochet wrote:
> > Hello Ludovic,
> >
> > On Tue, Jul 28, 2015 at 03:12:53PM +0200, Ludovic Desroches wrote:
> > > On Tue, Jul 28, 2015 at 02:48:09PM +0200, Linus Walleij wrote:
> > > > On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
>
> [...]
>
> > > > Is this a regression to v4.2 that should go to stable or v4.3 material?
> > >
> > > Yes it is a regression from v4.0, it applies well on v4.0.9
> > >
> > > Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
> > > Cc: stable at vger.kernel.org # 4.0
> >
> > a0b957f306fa have a stable tag up to 3.18, should this patch inherit a
> > stable tag up to 3.18 instead of only up to 4.0 ?
>
> Well spotted, I didn't notice it has hit 3.18 through stable, so yes
> stable tag should be 3.18 and later.
Fixes: a0b957f306fa ("pinctrl: at91: allow to have disabled gpio bank")
Cc: stable at vger.kernel.org # 3.18
Sylvain
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
2015-07-28 7:48 ` Ludovic Desroches
(?)
@ 2015-08-03 8:19 ` Linus Walleij
-1 siblings, 0 replies; 39+ messages in thread
From: Linus Walleij @ 2015-08-03 8:19 UTC (permalink / raw)
To: Ludovic Desroches
Cc: linux-gpio, Jean-Christophe PLAGNIOL-VILLARD, davidcdueck,
Alexandre Belloni, Nicolas Ferre, Boris Brezillon,
linux-arm-kernel, linux-kernel
On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
<ludovic.desroches@atmel.com> wrote:
> From: David Dueck <davidcdueck@googlemail.com>
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
(...)
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> ---
>
> This patch fixes a oops in the kernel because of a NULL pointer in a table.
> Having a NULL pointer in this table is the normal behavior if a PIO controller
> is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> an entry from a table if it is not filled.
This v2 version applied for fixes with v3.18+ stable tag.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-08-03 8:19 ` Linus Walleij
0 siblings, 0 replies; 39+ messages in thread
From: Linus Walleij @ 2015-08-03 8:19 UTC (permalink / raw)
To: Ludovic Desroches
Cc: linux-gpio, Jean-Christophe PLAGNIOL-VILLARD, davidcdueck,
Alexandre Belloni, Nicolas Ferre, Boris Brezillon,
linux-arm-kernel, linux-kernel
On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
<ludovic.desroches@atmel.com> wrote:
> From: David Dueck <davidcdueck@googlemail.com>
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
(...)
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel@lists.infradead.org
> CC: linux-kernel@vger.kernel.org
> ---
>
> This patch fixes a oops in the kernel because of a NULL pointer in a table.
> Having a NULL pointer in this table is the normal behavior if a PIO controller
> is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> an entry from a table if it is not filled.
This v2 version applied for fixes with v3.18+ stable tag.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 39+ messages in thread
* [RESEND PATCH] pinctrl: at91: fix null pointer dereference
@ 2015-08-03 8:19 ` Linus Walleij
0 siblings, 0 replies; 39+ messages in thread
From: Linus Walleij @ 2015-08-03 8:19 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jul 28, 2015 at 9:48 AM, Ludovic Desroches
<ludovic.desroches@atmel.com> wrote:
> From: David Dueck <davidcdueck@googlemail.com>
>
> Not all gpio banks are necessarily enabled, in the current code this can
> lead to null pointer dereferences.
(...)
>
> Signed-off-by: David Dueck <davidcdueck@googlemail.com>
> Acked-by: Ludovic Desroches <ludovic.desroches@atmel.com>
> Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
> CC: Nicolas Ferre <nicolas.ferre@atmel.com>
> CC: Boris Brezillon <boris.brezillon@free-electrons.com>
> CC: linux-arm-kernel at lists.infradead.org
> CC: linux-kernel at vger.kernel.org
> ---
>
> This patch fixes a oops in the kernel because of a NULL pointer in a table.
> Having a NULL pointer in this table is the normal behavior if a PIO controller
> is not enabled. So this fix is not a quick and dirty hack, it's usual to skip
> an entry from a table if it is not filled.
This v2 version applied for fixes with v3.18+ stable tag.
Yours,
Linus Walleij
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2015-08-03 8:19 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-02 16:06 [PATCH] pinctrl: at91: fix null pointer dereference David Dueck
2015-07-02 16:06 ` David Dueck
2015-07-03 9:38 ` Ludovic Desroches
2015-07-03 9:38 ` Ludovic Desroches
2015-07-03 9:38 ` Ludovic Desroches
2015-07-07 17:02 ` Alexandre Belloni
2015-07-07 17:02 ` Alexandre Belloni
2015-07-13 6:14 ` Jean-Christophe PLAGNIOL-VILLARD
2015-07-13 6:14 ` Jean-Christophe PLAGNIOL-VILLARD
2015-07-13 13:00 ` Ludovic Desroches
2015-07-13 13:00 ` Ludovic Desroches
2015-07-13 20:30 ` Jean-Christophe PLAGNIOL-VILLARD
2015-07-13 20:30 ` Jean-Christophe PLAGNIOL-VILLARD
2015-07-15 13:30 ` Ludovic Desroches
2015-07-15 13:30 ` Ludovic Desroches
2015-07-28 7:48 ` [RESEND PATCH] " Ludovic Desroches
2015-07-28 7:48 ` Ludovic Desroches
2015-07-28 7:48 ` Ludovic Desroches
2015-07-28 8:00 ` Nicolas Ferre
2015-07-28 8:00 ` Nicolas Ferre
2015-07-28 8:00 ` Nicolas Ferre
2015-07-28 12:48 ` Linus Walleij
2015-07-28 12:48 ` Linus Walleij
2015-07-28 12:48 ` Linus Walleij
2015-07-28 13:12 ` Ludovic Desroches
2015-07-28 13:12 ` Ludovic Desroches
2015-07-28 13:12 ` Ludovic Desroches
2015-07-30 8:33 ` Sylvain Rochet
2015-07-30 8:33 ` Sylvain Rochet
2015-07-30 8:33 ` Sylvain Rochet
2015-07-30 8:49 ` Ludovic Desroches
2015-07-30 8:49 ` Ludovic Desroches
2015-07-30 8:49 ` Ludovic Desroches
2015-07-30 9:16 ` Sylvain Rochet
2015-07-30 9:16 ` Sylvain Rochet
2015-07-30 9:16 ` Sylvain Rochet
2015-08-03 8:19 ` Linus Walleij
2015-08-03 8:19 ` Linus Walleij
2015-08-03 8:19 ` Linus Walleij
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.