[PATCH 0/3] fix a couple of atm->phy_data related issues

STALE2017d

9 messages, 3 authors, 2021-03-08 · open the first message on its own page

[PATCH 0/3] fix a couple of atm->phy_data related issues

From: Tong Zhang <hidden>
Date: 2021-03-08 03:26:54 

there are two drivers(zatm and idt77252) using PRIV() (i.e. atm->phy_data)
to store private data, but the driver happens to populate wrong
pointers: atm->dev_data. which actually cause null-ptr-dereference in 
following PRIV(dev). This patch series attemps to fix those two issues
along with a typo in atm struct.

Tong Zhang (3):
  atm: fix a typo in the struct description
  atm: uPD98402: fix incorrect allocation
  atm: idt77252: fix null-ptr-dereference

 drivers/atm/idt77105.c | 4 ++--
 drivers/atm/uPD98402.c | 2 +-
 include/linux/atmdev.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH 3/3] atm: idt77252: fix null-ptr-dereference

From: Tong Zhang <hidden>
Date: 2021-03-08 03:28:11 

this one is similar to the phy_data allocation fix in uPD98402, the
driver allocate the idt77105_priv and store to dev_data but later
dereference using dev->dev_data, which will cause null-ptr-dereference.

fix this issue by changing dev_data to phy_data so that PRIV(dev) can
work correctly.

Signed-off-by: Tong Zhang <redacted>
---
 drivers/atm/idt77105.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/atm/idt77105.c b/drivers/atm/idt77105.c
index 3c081b6171a8..bfca7b8a6f31 100644
--- a/drivers/atm/idt77105.c
+++ b/drivers/atm/idt77105.c
@@ -262,7 +262,7 @@ static int idt77105_start(struct atm_dev *dev)
 {
 	unsigned long flags;
 
-	if (!(dev->dev_data = kmalloc(sizeof(struct idt77105_priv),GFP_KERNEL)))
+	if (!(dev->phy_data = kmalloc(sizeof(struct idt77105_priv),GFP_KERNEL)))
 		return -ENOMEM;
 	PRIV(dev)->dev = dev;
 	spin_lock_irqsave(&idt77105_priv_lock, flags);
@@ -337,7 +337,7 @@ static int idt77105_stop(struct atm_dev *dev)
                 else
                     idt77105_all = walk->next;
 	        dev->phy = NULL;
-                dev->dev_data = NULL;
+                dev->phy_data = NULL;
                 kfree(walk);
                 break;
             }
-- 
2.25.1

[PATCH 1/3] atm: fix a typo in the struct description

From: Tong Zhang <hidden>
Date: 2021-03-08 03:28:11 

phy_data means private PHY data not date

Signed-off-by: Tong Zhang <redacted>
---
 include/linux/atmdev.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/atmdev.h b/include/linux/atmdev.h
index 60cd25c0461b..9b02961d65ee 100644
--- a/include/linux/atmdev.h
+++ b/include/linux/atmdev.h
@@ -151,7 +151,7 @@ struct atm_dev {
 	const char	*type;		/* device type name */
 	int		number;		/* device index */
 	void		*dev_data;	/* per-device data */
-	void		*phy_data;	/* private PHY date */
+	void		*phy_data;	/* private PHY data */
 	unsigned long	flags;		/* device flags (ATM_DF_*) */
 	struct list_head local;		/* local ATM addresses */
 	struct list_head lecs;		/* LECS ATM addresses learned via ILMI */
-- 
2.25.1

[PATCH 2/3] atm: uPD98402: fix incorrect allocation

From: Tong Zhang <hidden>
Date: 2021-03-08 03:28:11 

dev->dev_data is set in zatm.c, calling zatm_start() will overwrite this
dev->dev_data in uPD98402_start() and a subsequent PRIV(dev)->lock
(i.e dev->phy_data->lock) will result in a null-ptr-dereference.

I believe this is a typo and what it actually want to do is to allocate
phy_data instead of dev_data.

Signed-off-by: Tong Zhang <redacted>
---
 drivers/atm/uPD98402.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/atm/uPD98402.c b/drivers/atm/uPD98402.c
index 7850758b5bb8..239852d85558 100644
--- a/drivers/atm/uPD98402.c
+++ b/drivers/atm/uPD98402.c
@@ -211,7 +211,7 @@ static void uPD98402_int(struct atm_dev *dev)
 static int uPD98402_start(struct atm_dev *dev)
 {
 	DPRINTK("phy_start\n");
-	if (!(dev->dev_data = kmalloc(sizeof(struct uPD98402_priv),GFP_KERNEL)))
+	if (!(dev->phy_data = kmalloc(sizeof(struct uPD98402_priv),GFP_KERNEL)))
 		return -ENOMEM;
 	spin_lock_init(&PRIV(dev)->lock);
 	memset(&PRIV(dev)->sonet_stats,0,sizeof(struct k_sonet_stats));
-- 
2.25.1

Re: [PATCH 0/3] fix a couple of atm->phy_data related issues

From: Alexander Duyck <hidden>
Date: 2021-03-08 17:48:16 

On Mon, Mar 8, 2021 at 12:39 AM Tong Zhang [off-list ref] wrote:
there are two drivers(zatm and idt77252) using PRIV() (i.e. atm->phy_data)
to store private data, but the driver happens to populate wrong
pointers: atm->dev_data. which actually cause null-ptr-dereference in
following PRIV(dev). This patch series attemps to fix those two issues
along with a typo in atm struct.

Tong Zhang (3):
  atm: fix a typo in the struct description
  atm: uPD98402: fix incorrect allocation
  atm: idt77252: fix null-ptr-dereference

 drivers/atm/idt77105.c | 4 ++--
 drivers/atm/uPD98402.c | 2 +-
 include/linux/atmdev.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
For the 2 phys you actually seen null pointer dereferences or are your
changes based on just code review?

I ask because it seems like this code has been this way since 2005 and
in the case of uPD98402_start the code doesn't seem like it should
function the way it was as PRIV is phy_data and there being issues
seems pretty obvious since the initialization of things happens
immediately after the allocation.

I'm just wondering if it might make more sense to drop the code if it
hasn't been run in 15+ years rather than updating it?

Re: [PATCH 0/3] fix a couple of atm->phy_data related issues

From: Tong Zhang <hidden>
Date: 2021-03-08 17:55:54 

Hi Alex,
attached is the kernel log for zatm(uPD98402) -- I also have
idt77252's log -- which is similar to this one --
I think it makes sense to drop if no one is actually using it --
- Tong

[    5.740774] BUG: KASAN: null-ptr-deref in uPD98402_start+0x5e/0x219
[uPD98402]
[    5.741179] Write of size 4 at addr 000000000000002c by task modprobe/96
[    5.741548]
[    5.741637] CPU: 0 PID: 96 Comm: modprobe Not tainted 5.12.0-rc2-dirty #71
[    5.742017] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
BIOS rel-1.13.0-48-gd9c812dda519-prebuilt.qemu.org 04/01/2014
[    5.742635] Call Trace:
[    5.742775]  dump_stack+0x8a/0xb5
[    5.742966]  kasan_report.cold+0x10f/0x111
[    5.743197]  ? uPD98402_start+0x5e/0x219 [uPD98402]
[    5.743473]  uPD98402_start+0x5e/0x219 [uPD98402]
[    5.743739]  zatm_init_one+0x10b5/0x1311 [zatm]
[    5.743998]  ? zatm_int.cold+0x30/0x30 [zatm]
[    5.744246]  ? _raw_write_lock_irqsave+0xd0/0xd0
[    5.744507]  ? __mutex_lock_slowpath+0x10/0x10
[    5.744757]  ? _raw_spin_unlock_irqrestore+0xd/0x20
[    5.745030]  ? zatm_int.cold+0x30/0x30 [zatm]
[    5.745278]  local_pci_probe+0x6f/0xb0
[    5.745492]  pci_device_probe+0x171/0x240
[    5.745718]  ? pci_device_remove+0xe0/0xe0
[    5.745949]  ? kernfs_create_link+0xb6/0x110
[    5.746190]  ? sysfs_do_create_link_sd.isra.0+0x76/0xe0
[    5.746482]  really_probe+0x161/0x420
[    5.746691]  driver_probe_device+0x6d/0xd0
[    5.746923]  device_driver_attach+0x82/0x90
[    5.747158]  ? device_driver_attach+0x90/0x90
[    5.747402]  __driver_attach+0x60/0x100
[    5.747621]  ? device_driver_attach+0x90/0x90
[    5.747864]  bus_for_each_dev+0xe1/0x140
[    5.748075]  ? subsys_dev_iter_exit+0x10/0x10
[    5.748320]  ? klist_node_init+0x61/0x80
[    5.748542]  bus_add_driver+0x254/0x2a0
[    5.748760]  driver_register+0xd3/0x150
[    5.748977]  ? 0xffffffffc0030000
[    5.749163]  do_one_initcall+0x84/0x250
[    5.749380]  ? trace_event_raw_event_initcall_finish+0x150/0x150
[    5.749714]  ? _raw_spin_unlock_irqrestore+0xd/0x20
[    5.749987]  ? create_object+0x395/0x510
[    5.750210]  ? kasan_unpoison+0x21/0x50
[    5.750427]  do_init_module+0xf8/0x350
[    5.750640]  load_module+0x40c5/0x4410
[    5.750854]  ? module_frob_arch_sections+0x20/0x20
[    5.751123]  ? kernel_read_file+0x1cd/0x3e0
[    5.751364]  ? __do_sys_finit_module+0x108/0x170
[    5.751628]  __do_sys_finit_module+0x108/0x170
[    5.751879]  ? __ia32_sys_init_module+0x40/0x40
[    5.752126]  ? file_open_root+0x200/0x200
[    5.752353]  ? do_sys_open+0x85/0xe0
[    5.752556]  ? filp_open+0x50/0x50
[    5.752750]  ? fpregs_assert_state_consistent+0x4d/0x60
[    5.753042]  ? exit_to_user_mode_prepare+0x2f/0x130
[    5.753316]  do_syscall_64+0x33/0x40
[    5.753519]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[    5.753802] RIP: 0033:0x7ff64032dcf7
 ff c3 48 c7 c6 01 00 00 00 e9 a1
[    5.755029] RSP: 002b:00007ffd250ea358 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[    5.755449] RAX: ffffffffffffffda RBX: 0000000001093a70 RCX: 00007ff64032dcf7
[    5.755847] RDX: 0000000000000000 RSI: 00000000010929e0 RDI: 0000000000000003
[    5.756242] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000001
[    5.756635] R10: 00007ff640391300 R11: 0000000000000246 R12: 00000000010929e0
[    5.757029] R13: 0000000000000000 R14: 0000000001092dd0 R15: 0000000000000001

On Mon, Mar 8, 2021 at 12:47 PM Alexander Duyck
[off-list ref] wrote:
On Mon, Mar 8, 2021 at 12:39 AM Tong Zhang [off-list ref] wrote:
quoted
there are two drivers(zatm and idt77252) using PRIV() (i.e. atm->phy_data)
to store private data, but the driver happens to populate wrong
pointers: atm->dev_data. which actually cause null-ptr-dereference in
following PRIV(dev). This patch series attemps to fix those two issues
along with a typo in atm struct.

Tong Zhang (3):
  atm: fix a typo in the struct description
  atm: uPD98402: fix incorrect allocation
  atm: idt77252: fix null-ptr-dereference

 drivers/atm/idt77105.c | 4 ++--
 drivers/atm/uPD98402.c | 2 +-
 include/linux/atmdev.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
For the 2 phys you actually seen null pointer dereferences or are your
changes based on just code review?

I ask because it seems like this code has been this way since 2005 and
in the case of uPD98402_start the code doesn't seem like it should
function the way it was as PRIV is phy_data and there being issues
seems pretty obvious since the initialization of things happens
immediately after the allocation.

I'm just wondering if it might make more sense to drop the code if it
hasn't been run in 15+ years rather than updating it?

Re: [PATCH 0/3] fix a couple of atm->phy_data related issues

From: Alexander Duyck <hidden>
Date: 2021-03-08 18:07:54 

Hi Tong,

Is this direct-assigned hardware or is QEMU being used to emulate the
hardware here? Admittedly I don't know that much about ATM, so I am
not sure when/if those phys would have gone out of production. However
since the code dates back to 2005 I am guessing it is on the old side.

Ultimately the decision is up to Chas. However if there has been code
in place for this long that would trigger this kind of null pointer
dereference then it kind of points to the fact that those phys have
probably not been in use since at least back when Linus switched over
to git in 2005.

Thanks,

- Alex

On Mon, Mar 8, 2021 at 9:55 AM Tong Zhang [off-list ref] wrote:
Hi Alex,
attached is the kernel log for zatm(uPD98402) -- I also have
idt77252's log -- which is similar to this one --
I think it makes sense to drop if no one is actually using it --
- Tong

[    5.740774] BUG: KASAN: null-ptr-deref in uPD98402_start+0x5e/0x219
[uPD98402]
[    5.741179] Write of size 4 at addr 000000000000002c by task modprobe/96
[    5.741548]
[    5.741637] CPU: 0 PID: 96 Comm: modprobe Not tainted 5.12.0-rc2-dirty #71
[    5.742017] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
BIOS rel-1.13.0-48-gd9c812dda519-prebuilt.qemu.org 04/01/2014
[    5.742635] Call Trace:
[    5.742775]  dump_stack+0x8a/0xb5
[    5.742966]  kasan_report.cold+0x10f/0x111
[    5.743197]  ? uPD98402_start+0x5e/0x219 [uPD98402]
[    5.743473]  uPD98402_start+0x5e/0x219 [uPD98402]
[    5.743739]  zatm_init_one+0x10b5/0x1311 [zatm]
[    5.743998]  ? zatm_int.cold+0x30/0x30 [zatm]
[    5.744246]  ? _raw_write_lock_irqsave+0xd0/0xd0
[    5.744507]  ? __mutex_lock_slowpath+0x10/0x10
[    5.744757]  ? _raw_spin_unlock_irqrestore+0xd/0x20
[    5.745030]  ? zatm_int.cold+0x30/0x30 [zatm]
[    5.745278]  local_pci_probe+0x6f/0xb0
[    5.745492]  pci_device_probe+0x171/0x240
[    5.745718]  ? pci_device_remove+0xe0/0xe0
[    5.745949]  ? kernfs_create_link+0xb6/0x110
[    5.746190]  ? sysfs_do_create_link_sd.isra.0+0x76/0xe0
[    5.746482]  really_probe+0x161/0x420
[    5.746691]  driver_probe_device+0x6d/0xd0
[    5.746923]  device_driver_attach+0x82/0x90
[    5.747158]  ? device_driver_attach+0x90/0x90
[    5.747402]  __driver_attach+0x60/0x100
[    5.747621]  ? device_driver_attach+0x90/0x90
[    5.747864]  bus_for_each_dev+0xe1/0x140
[    5.748075]  ? subsys_dev_iter_exit+0x10/0x10
[    5.748320]  ? klist_node_init+0x61/0x80
[    5.748542]  bus_add_driver+0x254/0x2a0
[    5.748760]  driver_register+0xd3/0x150
[    5.748977]  ? 0xffffffffc0030000
[    5.749163]  do_one_initcall+0x84/0x250
[    5.749380]  ? trace_event_raw_event_initcall_finish+0x150/0x150
[    5.749714]  ? _raw_spin_unlock_irqrestore+0xd/0x20
[    5.749987]  ? create_object+0x395/0x510
[    5.750210]  ? kasan_unpoison+0x21/0x50
[    5.750427]  do_init_module+0xf8/0x350
[    5.750640]  load_module+0x40c5/0x4410
[    5.750854]  ? module_frob_arch_sections+0x20/0x20
[    5.751123]  ? kernel_read_file+0x1cd/0x3e0
[    5.751364]  ? __do_sys_finit_module+0x108/0x170
[    5.751628]  __do_sys_finit_module+0x108/0x170
[    5.751879]  ? __ia32_sys_init_module+0x40/0x40
[    5.752126]  ? file_open_root+0x200/0x200
[    5.752353]  ? do_sys_open+0x85/0xe0
[    5.752556]  ? filp_open+0x50/0x50
[    5.752750]  ? fpregs_assert_state_consistent+0x4d/0x60
[    5.753042]  ? exit_to_user_mode_prepare+0x2f/0x130
[    5.753316]  do_syscall_64+0x33/0x40
[    5.753519]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[    5.753802] RIP: 0033:0x7ff64032dcf7
 ff c3 48 c7 c6 01 00 00 00 e9 a1
[    5.755029] RSP: 002b:00007ffd250ea358 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[    5.755449] RAX: ffffffffffffffda RBX: 0000000001093a70 RCX: 00007ff64032dcf7
[    5.755847] RDX: 0000000000000000 RSI: 00000000010929e0 RDI: 0000000000000003
[    5.756242] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000001
[    5.756635] R10: 00007ff640391300 R11: 0000000000000246 R12: 00000000010929e0
[    5.757029] R13: 0000000000000000 R14: 0000000001092dd0 R15: 0000000000000001

On Mon, Mar 8, 2021 at 12:47 PM Alexander Duyck
[off-list ref] wrote:
quoted
On Mon, Mar 8, 2021 at 12:39 AM Tong Zhang [off-list ref] wrote:
quoted
there are two drivers(zatm and idt77252) using PRIV() (i.e. atm->phy_data)
to store private data, but the driver happens to populate wrong
pointers: atm->dev_data. which actually cause null-ptr-dereference in
following PRIV(dev). This patch series attemps to fix those two issues
along with a typo in atm struct.

Tong Zhang (3):
  atm: fix a typo in the struct description
  atm: uPD98402: fix incorrect allocation
  atm: idt77252: fix null-ptr-dereference

 drivers/atm/idt77105.c | 4 ++--
 drivers/atm/uPD98402.c | 2 +-
 include/linux/atmdev.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
For the 2 phys you actually seen null pointer dereferences or are your
changes based on just code review?

I ask because it seems like this code has been this way since 2005 and
in the case of uPD98402_start the code doesn't seem like it should
function the way it was as PRIV is phy_data and there being issues
seems pretty obvious since the initialization of things happens
immediately after the allocation.

I'm just wondering if it might make more sense to drop the code if it
hasn't been run in 15+ years rather than updating it?

Re: [PATCH 0/3] fix a couple of atm->phy_data related issues

From: Tong Zhang <hidden>
Date: 2021-03-08 18:20:55 

I have this emulated device in QEMU,
-- and I agree with you that probably no one has been using it for a while
IMHO, given the quality of the driver it also make sense to drop the
support completely
or we at least need to fix some obvious issues here.
Best,
- Tong

On Mon, Mar 8, 2021 at 1:06 PM Alexander Duyck
[off-list ref] wrote:
Hi Tong,

Is this direct-assigned hardware or is QEMU being used to emulate the
hardware here? Admittedly I don't know that much about ATM, so I am
not sure when/if those phys would have gone out of production. However
since the code dates back to 2005 I am guessing it is on the old side.

Ultimately the decision is up to Chas. However if there has been code
in place for this long that would trigger this kind of null pointer
dereference then it kind of points to the fact that those phys have
probably not been in use since at least back when Linus switched over
to git in 2005.

Thanks,

- Alex

On Mon, Mar 8, 2021 at 9:55 AM Tong Zhang [off-list ref] wrote:
quoted
Hi Alex,
attached is the kernel log for zatm(uPD98402) -- I also have
idt77252's log -- which is similar to this one --
I think it makes sense to drop if no one is actually using it --
- Tong

[    5.740774] BUG: KASAN: null-ptr-deref in uPD98402_start+0x5e/0x219
[uPD98402]
[    5.741179] Write of size 4 at addr 000000000000002c by task modprobe/96
[    5.741548]
[    5.741637] CPU: 0 PID: 96 Comm: modprobe Not tainted 5.12.0-rc2-dirty #71
[    5.742017] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009),
BIOS rel-1.13.0-48-gd9c812dda519-prebuilt.qemu.org 04/01/2014
[    5.742635] Call Trace:
[    5.742775]  dump_stack+0x8a/0xb5
[    5.742966]  kasan_report.cold+0x10f/0x111
[    5.743197]  ? uPD98402_start+0x5e/0x219 [uPD98402]
[    5.743473]  uPD98402_start+0x5e/0x219 [uPD98402]
[    5.743739]  zatm_init_one+0x10b5/0x1311 [zatm]
[    5.743998]  ? zatm_int.cold+0x30/0x30 [zatm]
[    5.744246]  ? _raw_write_lock_irqsave+0xd0/0xd0
[    5.744507]  ? __mutex_lock_slowpath+0x10/0x10
[    5.744757]  ? _raw_spin_unlock_irqrestore+0xd/0x20
[    5.745030]  ? zatm_int.cold+0x30/0x30 [zatm]
[    5.745278]  local_pci_probe+0x6f/0xb0
[    5.745492]  pci_device_probe+0x171/0x240
[    5.745718]  ? pci_device_remove+0xe0/0xe0
[    5.745949]  ? kernfs_create_link+0xb6/0x110
[    5.746190]  ? sysfs_do_create_link_sd.isra.0+0x76/0xe0
[    5.746482]  really_probe+0x161/0x420
[    5.746691]  driver_probe_device+0x6d/0xd0
[    5.746923]  device_driver_attach+0x82/0x90
[    5.747158]  ? device_driver_attach+0x90/0x90
[    5.747402]  __driver_attach+0x60/0x100
[    5.747621]  ? device_driver_attach+0x90/0x90
[    5.747864]  bus_for_each_dev+0xe1/0x140
[    5.748075]  ? subsys_dev_iter_exit+0x10/0x10
[    5.748320]  ? klist_node_init+0x61/0x80
[    5.748542]  bus_add_driver+0x254/0x2a0
[    5.748760]  driver_register+0xd3/0x150
[    5.748977]  ? 0xffffffffc0030000
[    5.749163]  do_one_initcall+0x84/0x250
[    5.749380]  ? trace_event_raw_event_initcall_finish+0x150/0x150
[    5.749714]  ? _raw_spin_unlock_irqrestore+0xd/0x20
[    5.749987]  ? create_object+0x395/0x510
[    5.750210]  ? kasan_unpoison+0x21/0x50
[    5.750427]  do_init_module+0xf8/0x350
[    5.750640]  load_module+0x40c5/0x4410
[    5.750854]  ? module_frob_arch_sections+0x20/0x20
[    5.751123]  ? kernel_read_file+0x1cd/0x3e0
[    5.751364]  ? __do_sys_finit_module+0x108/0x170
[    5.751628]  __do_sys_finit_module+0x108/0x170
[    5.751879]  ? __ia32_sys_init_module+0x40/0x40
[    5.752126]  ? file_open_root+0x200/0x200
[    5.752353]  ? do_sys_open+0x85/0xe0
[    5.752556]  ? filp_open+0x50/0x50
[    5.752750]  ? fpregs_assert_state_consistent+0x4d/0x60
[    5.753042]  ? exit_to_user_mode_prepare+0x2f/0x130
[    5.753316]  do_syscall_64+0x33/0x40
[    5.753519]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[    5.753802] RIP: 0033:0x7ff64032dcf7
 ff c3 48 c7 c6 01 00 00 00 e9 a1
[    5.755029] RSP: 002b:00007ffd250ea358 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[    5.755449] RAX: ffffffffffffffda RBX: 0000000001093a70 RCX: 00007ff64032dcf7
[    5.755847] RDX: 0000000000000000 RSI: 00000000010929e0 RDI: 0000000000000003
[    5.756242] RBP: 0000000000000003 R08: 0000000000000000 R09: 0000000000000001
[    5.756635] R10: 00007ff640391300 R11: 0000000000000246 R12: 00000000010929e0
[    5.757029] R13: 0000000000000000 R14: 0000000001092dd0 R15: 0000000000000001

On Mon, Mar 8, 2021 at 12:47 PM Alexander Duyck
[off-list ref] wrote:
quoted
On Mon, Mar 8, 2021 at 12:39 AM Tong Zhang [off-list ref] wrote:
quoted
there are two drivers(zatm and idt77252) using PRIV() (i.e. atm->phy_data)
to store private data, but the driver happens to populate wrong
pointers: atm->dev_data. which actually cause null-ptr-dereference in
following PRIV(dev). This patch series attemps to fix those two issues
along with a typo in atm struct.

Tong Zhang (3):
  atm: fix a typo in the struct description
  atm: uPD98402: fix incorrect allocation
  atm: idt77252: fix null-ptr-dereference

 drivers/atm/idt77105.c | 4 ++--
 drivers/atm/uPD98402.c | 2 +-
 include/linux/atmdev.h | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)
For the 2 phys you actually seen null pointer dereferences or are your
changes based on just code review?

I ask because it seems like this code has been this way since 2005 and
in the case of uPD98402_start the code doesn't seem like it should
function the way it was as PRIV is phy_data and there being issues
seems pretty obvious since the initialization of things happens
immediately after the allocation.

I'm just wondering if it might make more sense to drop the code if it
hasn't been run in 15+ years rather than updating it?

Re: [PATCH 0/3] fix a couple of atm->phy_data related issues

From: patchwork-bot+netdevbpf@kernel.org
Date: 2021-03-08 23:30:41 

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Sun,  7 Mar 2021 22:25:27 -0500 you wrote:
there are two drivers(zatm and idt77252) using PRIV() (i.e. atm->phy_data)
to store private data, but the driver happens to populate wrong
pointers: atm->dev_data. which actually cause null-ptr-dereference in
following PRIV(dev). This patch series attemps to fix those two issues
along with a typo in atm struct.

Tong Zhang (3):
  atm: fix a typo in the struct description
  atm: uPD98402: fix incorrect allocation
  atm: idt77252: fix null-ptr-dereference

[...]
Here is the summary with links:
  - [1/3] atm: fix a typo in the struct description
    https://git.kernel.org/netdev/net/c/1019d7923d9d
  - [2/3] atm: uPD98402: fix incorrect allocation
    https://git.kernel.org/netdev/net/c/3153724fc084
  - [3/3] atm: idt77252: fix null-ptr-dereference
    https://git.kernel.org/netdev/net/c/4416e98594dc

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help