[syzbot] [dri?] general protection fault in drm_object_attach_property

[syzbot] [dri?] general protection fault in drm_object_attach_property

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    3d6d817622b0 Merge tag 'scsi-fixes' of git://git.kernel.or..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15432c79580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=8a233ed2ce9a8096
dashboard link: https://syzkaller.appspot.com/bug?extid=1944765c3659f63d3777
compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
userspace arch: i386
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=10b78ac6580000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=14735149580000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/d900f083ada3/non_bootable_disk-3d6d8176.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/b5f85cb20f42/vmlinux-3d6d8176.xz
kernel image: https://storage.googleapis.com/syzbot-assets/8ee2de80a3a1/bzImage-3d6d8176.xz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com

usb 5-1: config 1 interface 0 altsetting 0 bulk endpoint 0x1 has invalid maxpacket 0
usb 5-1: config 1 interface 0 altsetting 0 has 1 endpoint descriptor, different from the interface descriptor's value: 2
usb 5-1: New USB device strings: Mfr=0, Product=0, SerialNumber=0
Oops: general protection fault, probably for non-canonical address 0xdffffc000000000c: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000060-0x0000000000000067]
CPU: 2 UID: 0 PID: 852 Comm: kworker/2:2 Not tainted syzkaller #0 PREEMPT(full) 
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
Workqueue: usb_hub_wq hub_event
RIP: 0010:drm_object_attach_property+0x85/0x3b0 drivers/gpu/drm/drm_mode_object.c:240
Code: 0f b6 04 02 84 c0 74 08 3c 03 0f 8e 93 02 00 00 49 8d 7c 24 60 49 63 5d 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 a2 02 00 00 48 8d 7d 04 4d 8b 74 24 60 48 b8 00
RSP: 0000:ffffc900038f6e60 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: 0000000000000007 RCX: ffffffff86574465
RDX: 000000000000000c RSI: ffffffff85c3beb2 RDI: 0000000000000060
RBP: ffff888012bfe040 R08: 0000000000000007 R09: fffffffffffff000
R10: ffff888012bfece4 R11: 0000000000000000 R12: 0000000000000000
R13: ffff888012bfe228 R14: ffff888012bfece4 R15: 0000000000000000
FS:  0000000000000000(0000) GS:ffff888096df3000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000561c6bdd7168 CR3: 000000005d115000 CR4: 0000000000352ef0
Call Trace:
 <TASK>
 gud_connector_add_properties drivers/gpu/drm/gud/gud_connector.c:572 [inline]
 gud_connector_create drivers/gpu/drm/gud/gud_connector.c:680 [inline]
 gud_get_connectors+0x86e/0x1700 drivers/gpu/drm/gud/gud_connector.c:717
 gud_probe+0x17aa/0x1c20 drivers/gpu/drm/gud/gud_drv.c:635
 usb_probe_interface+0x303/0x8f0 drivers/usb/core/driver.c:396
 call_driver_probe drivers/base/dd.c:628 [inline]
 really_probe+0x241/0xa60 drivers/base/dd.c:706
 __driver_probe_device+0x20e/0x450 drivers/base/dd.c:868
 driver_probe_device+0x4a/0x140 drivers/base/dd.c:898
 __device_attach_driver+0x1df/0x320 drivers/base/dd.c:1026
 bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:500
 __device_attach+0x1e4/0x4d0 drivers/base/dd.c:1098
 device_initial_probe+0xaf/0xd0 drivers/base/dd.c:1153
 bus_probe_device+0x64/0x160 drivers/base/bus.c:620
 device_add+0x121d/0x1970 drivers/base/core.c:3772
 usb_set_configuration+0xd97/0x1c60 drivers/usb/core/message.c:2268
 usb_generic_driver_probe+0xa1/0xe0 drivers/usb/core/generic.c:250
 usb_probe_device+0xef/0x400 drivers/usb/core/driver.c:291
 call_driver_probe drivers/base/dd.c:628 [inline]
 really_probe+0x241/0xa60 drivers/base/dd.c:706
 __driver_probe_device+0x20e/0x450 drivers/base/dd.c:868
 driver_probe_device+0x4a/0x140 drivers/base/dd.c:898
 __device_attach_driver+0x1df/0x320 drivers/base/dd.c:1026
 bus_for_each_drv+0x159/0x1e0 drivers/base/bus.c:500
 __device_attach+0x1e4/0x4d0 drivers/base/dd.c:1098
 device_initial_probe+0xaf/0xd0 drivers/base/dd.c:1153
 bus_probe_device+0x64/0x160 drivers/base/bus.c:620
 device_add+0x121d/0x1970 drivers/base/core.c:3772
 usb_new_device.part.0+0xcc2/0x1686 drivers/usb/core/hub.c:2703
 usb_new_device include/linux/workqueue.h:714 [inline]
 hub_port_connect drivers/usb/core/hub.c:5575 [inline]
 hub_port_connect_change drivers/usb/core/hub.c:5715 [inline]
 port_event drivers/usb/core/hub.c:5879 [inline]
 hub_event.cold+0x1e3/0xe90 drivers/usb/core/hub.c:5961
 process_one_work+0xa23/0x1940 kernel/workqueue.c:3322
 process_scheduled_works kernel/workqueue.c:3405 [inline]
 worker_thread+0x5ef/0xe50 kernel/workqueue.c:3486
 kthread+0x370/0x450 kernel/kthread.c:436
 ret_from_fork+0x72b/0xd50 arch/x86/kernel/process.c:158
 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
 </TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
RIP: 0010:drm_object_attach_property+0x85/0x3b0 drivers/gpu/drm/drm_mode_object.c:240
Code: 0f b6 04 02 84 c0 74 08 3c 03 0f 8e 93 02 00 00 49 8d 7c 24 60 49 63 5d 00 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 0f 85 a2 02 00 00 48 8d 7d 04 4d 8b 74 24 60 48 b8 00
RSP: 0000:ffffc900038f6e60 EFLAGS: 00010206
RAX: dffffc0000000000 RBX: 0000000000000007 RCX: ffffffff86574465
RDX: 000000000000000c RSI: ffffffff85c3beb2 RDI: 0000000000000060
RBP: ffff888012bfe040 R08: 0000000000000007 R09: fffffffffffff000
R10: ffff888012bfece4 R11: 0000000000000000 R12: 0000000000000000
R13: ffff888012bfe228 R14: ffff888012bfece4 R15: 0000000000000000
FS:  0000000000000000(0000) GS:ffff888096df3000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000561c6bdd7168 CR3: 000000005d115000 CR4: 0000000000352ef0
----------------
Code disassembly (best guess):
   0:	0f b6 04 02          	movzbl (%rdx,%rax,1),%eax
   4:	84 c0                	test   %al,%al
   6:	74 08                	je     0x10
   8:	3c 03                	cmp    $0x3,%al
   a:	0f 8e 93 02 00 00    	jle    0x2a3
  10:	49 8d 7c 24 60       	lea    0x60(%r12),%rdi
  15:	49 63 5d 00          	movslq 0x0(%r13),%rbx
  19:	48 b8 00 00 00 00 00 	movabs $0xdffffc0000000000,%rax
  20:	fc ff df
  23:	48 89 fa             	mov    %rdi,%rdx
  26:	48 c1 ea 03          	shr    $0x3,%rdx
* 2a:	80 3c 02 00          	cmpb   $0x0,(%rdx,%rax,1) <-- trapping instruction
  2e:	0f 85 a2 02 00 00    	jne    0x2d6
  34:	48 8d 7d 04          	lea    0x4(%rbp),%rdi
  38:	4d 8b 74 24 60       	mov    0x60(%r12),%r14
  3d:	48                   	rex.W
  3e:	b8                   	.byte 0xb8


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: [syzbot] [dri?] general protection fault in drm_object_attach_property

[Edward Adam Davis]

#syz test

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 11646453aaac..ecbbc967f51f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2175,8 +2175,12 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	struct drm_property *tv_subconnector;
 	unsigned int i;
 
-	if (dev->mode_config.tv_select_subconnector_property)
+	if (dev->mode_config.tv_select_subconnector_property) {
+		if (num_modes && !dev->mode_config.legacy_tv_mode_property)
+			goto other;
+
 		return 0;
+	}
 
 	/*
 	 * Basic connector properties
@@ -2199,6 +2203,7 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 		goto nomem;
 	dev->mode_config.tv_subconnector_property = tv_subconnector;
 
+other:
 	/*
 	 * Other, TV specific properties: margins & TV modes.
 	 */

Re: [syzbot] [dri?] general protection fault in drm_object_attach_property

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com
Tested-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com

Tested on:

commit:         8d3ae592 Linux 7.2
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=13a7d6c6580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=8a233ed2ce9a8096
dashboard link: https://syzkaller.appspot.com/bug?extid=1944765c3659f63d3777
compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
userspace arch: i386
patch:          https://syzkaller.appspot.com/x/patch.diff?x=10e92679580000

Note: testing is done by a robot and is best-effort only.

[PATCH] drm: Optimized by adding the TV modes attribute

[Edward Adam Davis]

When adding gud properties for drm connector within the function
gud_connector_add_properties(), if the TV modes property is not added
first, drm_mode_create_tv_properties_legacy() would fail to add the TV
modes property because the tv_select_subconnector_property has already
been added.

This causes gud_connector_property_lookup() to fail when looking for
the TV modes property (returning NULL), which subsequently triggers
issue [1] when a NULL property is passed to drm_object_attach_property().

The fix ensures that within drm_mode_create_tv_properties_legacy(), the
TV modes property is correctly added regardless of whether the subconnector
property exists.

[1]
Oops: general protection fault, probably for non-canonical address 0xdffffc000000000c: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000060-0x0000000000000067]
RIP: 0010:drm_object_attach_property+0x85/0x3b0 drivers/gpu/drm/drm_mode_object.c:240
Call Trace:
 gud_connector_add_properties drivers/gpu/drm/gud/gud_connector.c:572 [inline]
 gud_connector_create drivers/gpu/drm/gud/gud_connector.c:680 [inline]
 gud_get_connectors+0x86e/0x1700 drivers/gpu/drm/gud/gud_connector.c:717
 gud_probe+0x17aa/0x1c20 drivers/gpu/drm/gud/gud_drv.c:635
 
Fixes: f453ba046074 ("DRM: add mode setting support")
Reported-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1944765c3659f63d3777
Tested-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 drivers/gpu/drm/drm_connector.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 11646453aaac..ecbbc967f51f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2175,8 +2175,12 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	struct drm_property *tv_subconnector;
 	unsigned int i;
 
-	if (dev->mode_config.tv_select_subconnector_property)
+	if (dev->mode_config.tv_select_subconnector_property) {
+		if (num_modes && !dev->mode_config.legacy_tv_mode_property)
+			goto other;
+
 		return 0;
+	}
 
 	/*
 	 * Basic connector properties
@@ -2199,6 +2203,7 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 		goto nomem;
 	dev->mode_config.tv_subconnector_property = tv_subconnector;
 
+other:
 	/*
 	 * Other, TV specific properties: margins & TV modes.
 	 */
-- 
2.43.0

Re: [PATCH] drm: Optimized by adding the TV modes attribute

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unconditional recreation of DRM TV properties breaks atomic uAPI property updates and causes memory leaks of `drm_property` objects.

Pre-existing issues:
- [High] Out-of-bounds read in TV mode name processing via `strlen()` due to missing null-termination guarantees.
--

--- Patch [1]: [PATCH] drm: Optimized by adding the TV modes attribute ---
commit 5de13986f93b84f8b4c689b55a18d298dfc7431d
Author: Edward Adam Davis <eadavis@qq.com>

drm: Optimized by adding the TV modes attribute

When adding gud properties for drm connector within the function
gud_connector_add_properties(), if the TV modes property is not added
first, drm_mode_create_tv_properties_legacy() would fail to add the TV
modes property because the tv_select_subconnector_property has already
been added.

Closes: https://syzkaller.appspot.com/bug?extid=1944765c3659f63d3777

> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 8b4baed060f3a..7728c2941a701 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2201,8 +2201,12 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
>  	struct drm_property *tv_subconnector;
>  	unsigned int i;
>  
> -	if (dev->mode_config.tv_select_subconnector_property)
> +	if (dev->mode_config.tv_select_subconnector_property) {
> +		if (num_modes && !dev->mode_config.legacy_tv_mode_property)
> +			goto other;
> +
>  		return 0;
> +	}
>  
>  	/*
>  	 * Basic connector properties

[ ... ]

> @@ -2225,6 +2229,7 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
>  		goto nomem;
>  	dev->mode_config.tv_subconnector_property = tv_subconnector;
>  
> +other:
>  	/*
>  	 * Other, TV specific properties: margins & TV modes.
>  	 */

[Severity: High]
Does this goto path unconditionally recreate properties like
tv_brightness_property?

If a GUD USB device exposes multiple TV properties, the driver calls
drm_mode_create_tv_properties_legacy() multiple times. For subsequent
properties, the goto other path is hit, but it proceeds to unconditionally
call drm_property_create_range() for brightness, contrast, etc.

Would this overwrite the pointers in dev->mode_config and leak the orphaned
drm_property objects?

Furthermore, during a userspace atomic commit to update the brightness
property on the first connector, wouldn't the pointer mismatch cause the
kernel to reject the valid property update with -EINVAL?

drivers/gpu/drm/drm_atomic_uapi.c:drm_atomic_connector_set_property() {
	...
	} else if (property == config->tv_brightness_property) {
		state->tv.brightness = val;
	...
}

[Severity: High]
This isn't a bug introduced by this patch, but does the TV mode name
processing in gud_connector_add_tv_mode() have an out-of-bounds read?

Untrusted data is read from the USB device into buf without enforcing
null-termination:

drivers/gpu/drm/gud/gud_connector.c:gud_connector_add_tv_mode() {
	...
	ret = gud_usb_get(gdrm, GUD_REQ_GET_CONNECTOR_TV_MODE_VALUES,
			  connector->index, buf, buf_len);
	...
	for (i = 0; i < num_modes; i++)
		modes[i] = &buf[i * GUD_CONNECTOR_TV_MODE_NAME_LEN];
	...
}

If the USB device payload contains full 16-byte chunks without any null
terminators, would the strlen() in drm_property_add_enum() read past the
end of the kmalloced buffer?

drivers/gpu/drm/drm_property.c:drm_property_add_enum() {
	...
	if (WARN_ON(strlen(name) >= DRM_PROP_NAME_LEN))
		return -EINVAL;
	...
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_C6A625E0BAF4BB77334C9E0F1B244B7B1B08@qq.com?part=1

Re: [syzbot] [dri?] general protection fault in drm_object_attach_property

[Edward Adam Davis]

#syz test

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 11646453aaac..12f4c5fcec92 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2175,8 +2175,12 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	struct drm_property *tv_subconnector;
 	unsigned int i;
 
-	if (dev->mode_config.tv_select_subconnector_property)
+	if (dev->mode_config.tv_select_subconnector_property) {
+		if (num_modes && !dev->mode_config.legacy_tv_mode_property)
+			goto tv_mode;
+
 		return 0;
+	}
 
 	/*
 	 * Basic connector properties
@@ -2205,18 +2209,6 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	if (drm_mode_create_tv_margin_properties(dev))
 		goto nomem;
 
-	if (num_modes) {
-		dev->mode_config.legacy_tv_mode_property =
-			drm_property_create(dev, DRM_MODE_PROP_ENUM,
-					    "mode", num_modes);
-		if (!dev->mode_config.legacy_tv_mode_property)
-			goto nomem;
-
-		for (i = 0; i < num_modes; i++)
-			drm_property_add_enum(dev->mode_config.legacy_tv_mode_property,
-					      i, modes[i]);
-	}
-
 	dev->mode_config.tv_brightness_property =
 		drm_property_create_range(dev, 0, "brightness", 0, 100);
 	if (!dev->mode_config.tv_brightness_property)
@@ -2247,6 +2239,19 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	if (!dev->mode_config.tv_hue_property)
 		goto nomem;
 
+tv_mode:
+	if (num_modes) {
+		dev->mode_config.legacy_tv_mode_property =
+			drm_property_create(dev, DRM_MODE_PROP_ENUM,
+					    "mode", num_modes);
+		if (!dev->mode_config.legacy_tv_mode_property)
+			goto nomem;
+
+		for (i = 0; i < num_modes; i++)
+			drm_property_add_enum(dev->mode_config.legacy_tv_mode_property,
+					      i, modes[i]);
+	}
+
 	return 0;
 nomem:
 	return -ENOMEM;

Re: [syzbot] [dri?] general protection fault in drm_object_attach_property

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com
Tested-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com

Tested on:

commit:         8d3ae592 Linux 7.2
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=147f2679580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=8a233ed2ce9a8096
dashboard link: https://syzkaller.appspot.com/bug?extid=1944765c3659f63d3777
compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
userspace arch: i386
patch:          https://syzkaller.appspot.com/x/patch.diff?x=12af2679580000

Note: testing is done by a robot and is best-effort only.

Re: [syzbot] [dri?] general protection fault in drm_object_attach_property

[Edward Adam Davis]

#syz test

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 11646453aaac..800fcc44e3f4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2175,29 +2175,30 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	struct drm_property *tv_subconnector;
 	unsigned int i;
 
-	if (dev->mode_config.tv_select_subconnector_property)
-		return 0;
-
-	/*
-	 * Basic connector properties
-	 */
-	tv_selector = drm_property_create_enum(dev, 0,
-					  "select subconnector",
-					  drm_tv_select_enum_list,
-					  ARRAY_SIZE(drm_tv_select_enum_list));
-	if (!tv_selector)
-		goto nomem;
+	if (!dev->mode_config.tv_select_subconnector_property) {
+		/*
+		 * Basic connector properties
+		 */
+		tv_selector = drm_property_create_enum(dev, 0,
+					"select subconnector",
+					drm_tv_select_enum_list,
+					ARRAY_SIZE(drm_tv_select_enum_list));
+		if (!tv_selector)
+			goto nomem;
 
-	dev->mode_config.tv_select_subconnector_property = tv_selector;
+		dev->mode_config.tv_select_subconnector_property = tv_selector;
+	}
 
-	tv_subconnector =
-		drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
-				    "subconnector",
-				    drm_tv_subconnector_enum_list,
-				    ARRAY_SIZE(drm_tv_subconnector_enum_list));
-	if (!tv_subconnector)
-		goto nomem;
-	dev->mode_config.tv_subconnector_property = tv_subconnector;
+	if (!dev->mode_config.tv_subconnector_property) {
+		tv_subconnector =
+			drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
+				"subconnector",
+				drm_tv_subconnector_enum_list,
+				ARRAY_SIZE(drm_tv_subconnector_enum_list));
+		if (!tv_subconnector)
+			goto nomem;
+		dev->mode_config.tv_subconnector_property = tv_subconnector;
+	}
 
 	/*
 	 * Other, TV specific properties: margins & TV modes.
@@ -2205,7 +2206,7 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	if (drm_mode_create_tv_margin_properties(dev))
 		goto nomem;
 
-	if (num_modes) {
+	if (num_modes && !dev->mode_config.legacy_tv_mode_property) {
 		dev->mode_config.legacy_tv_mode_property =
 			drm_property_create(dev, DRM_MODE_PROP_ENUM,
 					    "mode", num_modes);
@@ -2217,35 +2218,48 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 					      i, modes[i]);
 	}
 
-	dev->mode_config.tv_brightness_property =
-		drm_property_create_range(dev, 0, "brightness", 0, 100);
-	if (!dev->mode_config.tv_brightness_property)
-		goto nomem;
+	if (!dev->mode_config.tv_brightness_property) {
+		dev->mode_config.tv_brightness_property =
+			drm_property_create_range(dev, 0, "brightness", 0, 100);
+		if (!dev->mode_config.tv_brightness_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_contrast_property =
-		drm_property_create_range(dev, 0, "contrast", 0, 100);
-	if (!dev->mode_config.tv_contrast_property)
-		goto nomem;
+	if (!dev->mode_config.tv_contrast_property) {
+		dev->mode_config.tv_contrast_property =
+			drm_property_create_range(dev, 0, "contrast", 0, 100);
+		if (!dev->mode_config.tv_contrast_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_flicker_reduction_property =
-		drm_property_create_range(dev, 0, "flicker reduction", 0, 100);
-	if (!dev->mode_config.tv_flicker_reduction_property)
-		goto nomem;
+	if (!dev->mode_config.tv_flicker_reduction_property) {
+		dev->mode_config.tv_flicker_reduction_property =
+			drm_property_create_range(dev, 0, "flicker reduction",
+					0, 100);
+		if (!dev->mode_config.tv_flicker_reduction_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_overscan_property =
-		drm_property_create_range(dev, 0, "overscan", 0, 100);
-	if (!dev->mode_config.tv_overscan_property)
-		goto nomem;
+	if (!dev->mode_config.tv_overscan_property) {
+		dev->mode_config.tv_overscan_property =
+			drm_property_create_range(dev, 0, "overscan", 0, 100);
+		if (!dev->mode_config.tv_overscan_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_saturation_property =
-		drm_property_create_range(dev, 0, "saturation", 0, 100);
-	if (!dev->mode_config.tv_saturation_property)
-		goto nomem;
+	if (!dev->mode_config.tv_saturation_property) {
+		dev->mode_config.tv_saturation_property =
+			drm_property_create_range(dev, 0, "saturation", 0, 100);
+		if (!dev->mode_config.tv_saturation_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_hue_property =
-		drm_property_create_range(dev, 0, "hue", 0, 100);
-	if (!dev->mode_config.tv_hue_property)
-		goto nomem;
+	if (!dev->mode_config.tv_hue_property) {
+		dev->mode_config.tv_hue_property =
+			drm_property_create_range(dev, 0, "hue", 0, 100);
+		if (!dev->mode_config.tv_hue_property)
+			goto nomem;
+	}
 
 	return 0;
 nomem:

Re: [syzbot] [dri?] general protection fault in drm_object_attach_property

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com
Tested-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com

Tested on:

commit:         8d3ae592 Linux 7.2
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12a93a79580000
kernel config:  https://syzkaller.appspot.com/x/.config?x=8a233ed2ce9a8096
dashboard link: https://syzkaller.appspot.com/bug?extid=1944765c3659f63d3777
compiler:       gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
userspace arch: i386
patch:          https://syzkaller.appspot.com/x/patch.diff?x=14313a79580000

Note: testing is done by a robot and is best-effort only.

[PATCH v2] drm: Optimize tv properties creation

[Edward Adam Davis]

When adding gud properties for drm connector within the function
gud_connector_add_properties(), if the TV modes property is not added
first, drm_mode_create_tv_properties_legacy() would fail to add the TV
modes property because the tv_select_subconnector_property has already
been added. Other properties (such as brightness, contrast, etc.) are
affected by the same issue.

This causes gud_connector_property_lookup() to fail when looking for
the TV modes property (returning NULL), which subsequently triggers
issue [1] when a NULL property is passed to drm_object_attach_property().

The fix ensures that within drm_mode_create_tv_properties_legacy(), the
TV modes property is correctly added regardless of whether the subconnector
property exists.

Recreation of properties brightness(contrast, flicker reduction, overscan,
saturation, hue) must be prevented.

[1]
Oops: general protection fault, probably for non-canonical address 0xdffffc000000000c: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000060-0x0000000000000067]
RIP: 0010:drm_object_attach_property+0x85/0x3b0 drivers/gpu/drm/drm_mode_object.c:240
Call Trace:
 gud_connector_add_properties drivers/gpu/drm/gud/gud_connector.c:572 [inline]
 gud_connector_create drivers/gpu/drm/gud/gud_connector.c:680 [inline]
 gud_get_connectors+0x86e/0x1700 drivers/gpu/drm/gud/gud_connector.c:717
 gud_probe+0x17aa/0x1c20 drivers/gpu/drm/gud/gud_drv.c:635

Fixes: f453ba046074 ("DRM: add mode setting support")
Reported-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=1944765c3659f63d3777
Tested-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
v1 -> v2: avoid recreate brightness/contrast/.../hue properties;
	  update subject and comments

 drivers/gpu/drm/drm_connector.c | 106 ++++++++++++++++++--------------
 1 file changed, 60 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 11646453aaac..800fcc44e3f4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -2175,29 +2175,30 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	struct drm_property *tv_subconnector;
 	unsigned int i;
 
-	if (dev->mode_config.tv_select_subconnector_property)
-		return 0;
-
-	/*
-	 * Basic connector properties
-	 */
-	tv_selector = drm_property_create_enum(dev, 0,
-					  "select subconnector",
-					  drm_tv_select_enum_list,
-					  ARRAY_SIZE(drm_tv_select_enum_list));
-	if (!tv_selector)
-		goto nomem;
+	if (!dev->mode_config.tv_select_subconnector_property) {
+		/*
+		 * Basic connector properties
+		 */
+		tv_selector = drm_property_create_enum(dev, 0,
+					"select subconnector",
+					drm_tv_select_enum_list,
+					ARRAY_SIZE(drm_tv_select_enum_list));
+		if (!tv_selector)
+			goto nomem;
 
-	dev->mode_config.tv_select_subconnector_property = tv_selector;
+		dev->mode_config.tv_select_subconnector_property = tv_selector;
+	}
 
-	tv_subconnector =
-		drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
-				    "subconnector",
-				    drm_tv_subconnector_enum_list,
-				    ARRAY_SIZE(drm_tv_subconnector_enum_list));
-	if (!tv_subconnector)
-		goto nomem;
-	dev->mode_config.tv_subconnector_property = tv_subconnector;
+	if (!dev->mode_config.tv_subconnector_property) {
+		tv_subconnector =
+			drm_property_create_enum(dev, DRM_MODE_PROP_IMMUTABLE,
+				"subconnector",
+				drm_tv_subconnector_enum_list,
+				ARRAY_SIZE(drm_tv_subconnector_enum_list));
+		if (!tv_subconnector)
+			goto nomem;
+		dev->mode_config.tv_subconnector_property = tv_subconnector;
+	}
 
 	/*
 	 * Other, TV specific properties: margins & TV modes.
@@ -2205,7 +2206,7 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 	if (drm_mode_create_tv_margin_properties(dev))
 		goto nomem;
 
-	if (num_modes) {
+	if (num_modes && !dev->mode_config.legacy_tv_mode_property) {
 		dev->mode_config.legacy_tv_mode_property =
 			drm_property_create(dev, DRM_MODE_PROP_ENUM,
 					    "mode", num_modes);
@@ -2217,35 +2218,48 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
 					      i, modes[i]);
 	}
 
-	dev->mode_config.tv_brightness_property =
-		drm_property_create_range(dev, 0, "brightness", 0, 100);
-	if (!dev->mode_config.tv_brightness_property)
-		goto nomem;
+	if (!dev->mode_config.tv_brightness_property) {
+		dev->mode_config.tv_brightness_property =
+			drm_property_create_range(dev, 0, "brightness", 0, 100);
+		if (!dev->mode_config.tv_brightness_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_contrast_property =
-		drm_property_create_range(dev, 0, "contrast", 0, 100);
-	if (!dev->mode_config.tv_contrast_property)
-		goto nomem;
+	if (!dev->mode_config.tv_contrast_property) {
+		dev->mode_config.tv_contrast_property =
+			drm_property_create_range(dev, 0, "contrast", 0, 100);
+		if (!dev->mode_config.tv_contrast_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_flicker_reduction_property =
-		drm_property_create_range(dev, 0, "flicker reduction", 0, 100);
-	if (!dev->mode_config.tv_flicker_reduction_property)
-		goto nomem;
+	if (!dev->mode_config.tv_flicker_reduction_property) {
+		dev->mode_config.tv_flicker_reduction_property =
+			drm_property_create_range(dev, 0, "flicker reduction",
+					0, 100);
+		if (!dev->mode_config.tv_flicker_reduction_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_overscan_property =
-		drm_property_create_range(dev, 0, "overscan", 0, 100);
-	if (!dev->mode_config.tv_overscan_property)
-		goto nomem;
+	if (!dev->mode_config.tv_overscan_property) {
+		dev->mode_config.tv_overscan_property =
+			drm_property_create_range(dev, 0, "overscan", 0, 100);
+		if (!dev->mode_config.tv_overscan_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_saturation_property =
-		drm_property_create_range(dev, 0, "saturation", 0, 100);
-	if (!dev->mode_config.tv_saturation_property)
-		goto nomem;
+	if (!dev->mode_config.tv_saturation_property) {
+		dev->mode_config.tv_saturation_property =
+			drm_property_create_range(dev, 0, "saturation", 0, 100);
+		if (!dev->mode_config.tv_saturation_property)
+			goto nomem;
+	}
 
-	dev->mode_config.tv_hue_property =
-		drm_property_create_range(dev, 0, "hue", 0, 100);
-	if (!dev->mode_config.tv_hue_property)
-		goto nomem;
+	if (!dev->mode_config.tv_hue_property) {
+		dev->mode_config.tv_hue_property =
+			drm_property_create_range(dev, 0, "hue", 0, 100);
+		if (!dev->mode_config.tv_hue_property)
+			goto nomem;
+	}
 
 	return 0;
 nomem:
-- 
2.43.0

Re: [PATCH] drm: Optimized by adding the TV modes attribute

[Ruben Wauters]

[-- Attachment #1: Type: text/plain, Size: 2760 bytes --]

On Mon, 2026-08-17 at 20:37 +0800, Edward Adam Davis wrote:
> When adding gud properties for drm connector within the function
> gud_connector_add_properties(), if the TV modes property is not added
> first, drm_mode_create_tv_properties_legacy() would fail to add the TV
> modes property because the tv_select_subconnector_property has already
> been added.
> 
> This causes gud_connector_property_lookup() to fail when looking for
> the TV modes property (returning NULL), which subsequently triggers
> issue [1] when a NULL property is passed to drm_object_attach_property().
> 
> The fix ensures that within drm_mode_create_tv_properties_legacy(), the
> TV modes property is correctly added regardless of whether the subconnector
> property exists.
> 
> [1]
> Oops: general protection fault, probably for non-canonical address 0xdffffc000000000c: 0000 [#1] SMP KASAN NOPTI
> KASAN: null-ptr-deref in range [0x0000000000000060-0x0000000000000067]
> RIP: 0010:drm_object_attach_property+0x85/0x3b0 drivers/gpu/drm/drm_mode_object.c:240
> Call Trace:
>  gud_connector_add_properties drivers/gpu/drm/gud/gud_connector.c:572 [inline]
>  gud_connector_create drivers/gpu/drm/gud/gud_connector.c:680 [inline]
>  gud_get_connectors+0x86e/0x1700 drivers/gpu/drm/gud/gud_connector.c:717
>  gud_probe+0x17aa/0x1c20 drivers/gpu/drm/gud/gud_drv.c:635
>  
> Fixes: f453ba046074 ("DRM: add mode setting support")
> Reported-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1944765c3659f63d3777
> Tested-by: syzbot+1944765c3659f63d3777@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
Acked-by: Ruben Wauters <rubenru09@aol.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 11646453aaac..ecbbc967f51f 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2175,8 +2175,12 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
>  	struct drm_property *tv_subconnector;
>  	unsigned int i;
>  
> -	if (dev->mode_config.tv_select_subconnector_property)
> +	if (dev->mode_config.tv_select_subconnector_property) {
> +		if (num_modes && !dev->mode_config.legacy_tv_mode_property)
> +			goto other;
> +
>  		return 0;
> +	}
>  
>  	/*
>  	 * Basic connector properties
> @@ -2199,6 +2203,7 @@ int drm_mode_create_tv_properties_legacy(struct drm_device *dev,
>  		goto nomem;
>  	dev->mode_config.tv_subconnector_property = tv_subconnector;
>  
> +other:
>  	/*
>  	 * Other, TV specific properties: margins & TV modes.
>  	 */

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]