[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z8cpH3twrgmR0TxI@fedora>
Date: Tue, 4 Mar 2025 17:23:59 +0100
From: José Expósito <jose.exposito89@...il.com>
To: Louis Chauvet <louis.chauvet@...tlin.com>
Cc: hamohammed.sa@...il.com, simona@...ll.ch, melissa.srw@...il.com,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
tzimmermann@...e.de, airlied@...il.com,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 03/16] drm/vkms: Allow to configure multiple planes
via configfs
On Tue, Mar 04, 2025 at 04:35:50PM +0100, Louis Chauvet wrote:
>
>
> Le 04/03/2025 à 15:54, José Expósito a écrit :
> > Hi Louis,
> >
> > On Mon, Mar 03, 2025 at 11:34:50AM +0100, Louis Chauvet wrote:
> > >
> > >
> > > Le 03/03/2025 à 09:50, José Expósito a écrit :
> > > > Hi Louis,
> > > >
> > > > On Fri, Feb 28, 2025 at 03:43:25PM +0100, Louis Chauvet wrote:
> > > > >
> > > > >
> > > > > Le 25/02/2025 à 18:59, José Expósito a écrit :
> > > > > > Create a default subgroup at /config/vkms/planes to allow to create as
> > > > > > many planes as required.
> > > > > >
> > > > > > Reviewed-by: Louis Chauvet <louis.chauvet@...tlin.com>
> > > > > > Co-developed-by: Louis Chauvet <louis.chauvet@...tlin.com>
> > > > > > Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> > > > > > Signed-off-by: José Expósito <jose.exposito89@...il.com>
> > > > > > [...]
> > > > > > diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> > > > > > index 92512d52ddae..4f9d3341e6c0 100644
> > > > > > --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> > > > > > +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> > > > > > [...]
> > > > > > +static void plane_release(struct config_item *item)
> > > > > > +{
> > > > > > + struct vkms_configfs_plane *plane;
> > > > > > + struct mutex *lock;
> > > > > > +
> > > > > > + plane = plane_item_to_vkms_configfs_plane(item);
> > > > > > + lock = &plane->dev->lock;
> > > > > > +
> > > > > > + guard(mutex)(lock);
> > > > > > + vkms_config_destroy_plane(plane->config);
> > > > > > + kfree(plane);
> > > > > > +}
> > > > >
> > > > > I just found a flaw in our work: there is currently no way to forbid the
> > > > > deletion of item/symlinks...
> > > > >
> > > > > If you do:
> > > > >
> > > > > modprobe vkms
> > > > > cd /sys/kernel/config/vkms/
> > > > > mkdir DEV
> > > > > mkdir DEV/connectors/CON
> > > > > mkdir DEV/planes/PLA
> > > > > mkdir DEV/crtcs/CRT
> > > > > mkdir DEV/encoders/ENC
> > > > > ln -s DEV/crtcs/CRT DEV/planes/PLA/possible_crtcs/
> > > > > ln -s DEV/crtcs/CRT DEV/encoders/ENC/possible_crtcs
> > > > > ln -s DEV/encoders/ENC DEV/connectors/CON/possible_encoders
> > > > > echo 1 > DEV/planes/PLA/type
> > > > > tree
> > > > > echo 1 > DEV/enabled
> > > > > modetest -M vkms
> > > > > => everything fine
> > > > >
> > > > > rm DEV/connectors/CON/possible_encoders/ENC
> > > > > rmdir DEV/connectors/CON
> > > > > modetest -M vkms
> > > > > => BUG: KASAN: slab-use-after-free
> >
> > I'm trying to reproduce this issue, but those commands don't show any BUG
> > in dmesg. This is my Kasan .config:
> >
> > CONFIG_HAVE_ARCH_KASAN=y
> > CONFIG_HAVE_ARCH_KASAN_VMALLOC=y
> > CONFIG_CC_HAS_KASAN_GENERIC=y
> > CONFIG_CC_HAS_KASAN_SW_TAGS=y
> > CONFIG_CC_HAS_WORKING_NOSANITIZE_ADDRESS=y
> > CONFIG_KASAN=y
> > CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX=y
> > CONFIG_KASAN_GENERIC=y
> > # CONFIG_KASAN_OUTLINE is not set
> > CONFIG_KASAN_INLINE=y
> > CONFIG_KASAN_STACK=y
> > CONFIG_KASAN_VMALLOC=y
> > # CONFIG_KASAN_KUNIT_TEST is not set
> > CONFIG_KASAN_EXTRA_INFO=y
> >
> > I tryed to delete even more items:
> >
> > root@...nel-dev:/sys/kernel/config/vkms# tree
> > .
> > └── DEV
> > ├── connectors
> > ├── crtcs
> > ├── enabled
> > ├── encoders
> > └── planes
> >
> > root@...nel-dev:/sys/kernel/config/vkms# cat DEV/enabled
> > 1
> >
> > And I still don't see any errors. Is it possible that we are running different
> > branches? Asking because of the failing IGT tests you reported. There seems to
> > be a difference in our code or setup that is creating these differences.
>
> I just re-applied your last vkms-config version and this series on top of
> drm-misc-next. See [1] for the exact commits.
>
> Argg sorry, I just noticed something: you need to disable the default vkms
> device (I had this option in my kernel command line...), otherwise modetest
> only use the first vkms gpu...
>
> I will check again the igt tests, but I don't think this is the same issue
> (it should not use the default device to test)
>
> So, with [1] and the defconfig below, I have this:
>
>
> 1 modprobe vkms create_default_dev=0
> 2 cd /sys/kernel/config/vkms/
> 3 mkdir DEV
> 4 mkdir DEV/connectors/CON
> 5 mkdir DEV/planes/PLA
> 6 mkdir DEV/crtcs/CRT
> 7 mkdir DEV/encoders/ENC
> 8 ln -s DEV/crtcs/CRT DEV/planes/PLA/possible_crtcs/
> 9 ln -s DEV/crtcs/CRT DEV/encoders/ENC/possible_crtcs
> 10 ln -s DEV/encoders/ENC DEV/connectors/CON/possible_encoders
> 11 echo 1 > DEV/planes/PLA/type
> 12 tree
> 13 echo 1 > DEV/enabled
> 14 modetest -M vkms
> 15 rm DEV/connectors/CON/possible_encoders/ENC
> 16 rmdir DEV/connectors/CON
> 17 modetest -M vkms
> KASAN: slab-use-after-free
>
>
> [1]:https://github.com/Fomys/linux/tree/20250225175936.7223-1-jose.exposito89%40gmail.com
Aha! Are you testing without a desktop environment running?
$ sudo systemctl isolate multi-user.target
Running that (^) command before yours gives me this use after free:
BUG: KASAN: slab-use-after-free in vkms_connector_detect+0x61/0x70 [vkms]
Is the same one you are seeing?
Looking at the connector_release() function in vkms_configfs.c, I see
that I'm freeing the configuration:
vkms_config_destroy_connector(connector->config);
And I don't think there is a reason to do it. vkms_config_destroy() in
device_release() will free everything once we are done.
I need to do more testing, but the solution might be that simple.
Good catch, I didn't test like that and I overlooked this issue :D
Jose
>
>
> ===== defconfig =====
>
> CONFIG_SYSVIPC=y
> CONFIG_CGROUPS=y
> CONFIG_KALLSYMS_ALL=y
> CONFIG_SMP=y
> CONFIG_HYPERVISOR_GUEST=y
> CONFIG_PARAVIRT=y
> # CONFIG_VIRTUALIZATION is not set
> CONFIG_JUMP_LABEL=y
> CONFIG_MODULES=y
> CONFIG_MODULE_UNLOAD=y
> CONFIG_NET=y
> CONFIG_PACKET=y
> # CONFIG_WIRELESS is not set
> CONFIG_NET_9P=y
> CONFIG_NET_9P_VIRTIO=y
> CONFIG_PCI=y
> CONFIG_DEVTMPFS=y
> CONFIG_DEVTMPFS_MOUNT=y
> CONFIG_VIRTIO_BLK=y
> # CONFIG_INTEL_MEI is not set
> CONFIG_NETDEVICES=y
> CONFIG_VIRTIO_NET=y
> # CONFIG_ETHERNET is not set
> # CONFIG_WLAN is not set
> CONFIG_INPUT_EVDEV=y
> CONFIG_SERIAL_8250=y
> CONFIG_SERIAL_8250_CONSOLE=y
> CONFIG_VIRTIO_CONSOLE=y
> CONFIG_HW_RANDOM_VIRTIO=m
> CONFIG_PTP_1588_CLOCK=y
> # CONFIG_HWMON is not set
> CONFIG_THERMAL_GOV_USER_SPACE=y
> CONFIG_DRM=y
> CONFIG_DRM_KUNIT_TEST=m
> CONFIG_DRM_VKMS=m
> CONFIG_DRM_VKMS_KUNIT_TEST=m
> # CONFIG_USB_SUPPORT is not set
> CONFIG_VIRTIO_PCI=y
> CONFIG_VIRTIO_BALLOON=y
> CONFIG_VIRTIO_INPUT=y
> CONFIG_VIRTIO_MMIO=y
> CONFIG_VIRTIO_MMIO_CMDLINE_DEVICES=y
> # CONFIG_SURFACE_PLATFORMS is not set
> CONFIG_EXT4_FS=y
> CONFIG_FUSE_FS=y
> CONFIG_VIRTIO_FS=y
> CONFIG_OVERLAY_FS=y
> CONFIG_TMPFS=y
> CONFIG_TMPFS_POSIX_ACL=y
> CONFIG_CONFIGFS_FS=y
> CONFIG_9P_FS=y
> CONFIG_CRYPTO=y
> CONFIG_CRYPTO_CRC32C=y
> CONFIG_DYNAMIC_DEBUG=y
> CONFIG_DEBUG_KERNEL=y
> CONFIG_DEBUG_INFO_DWARF5=y
> CONFIG_MAGIC_SYSRQ=y
> CONFIG_DEBUG_FS=y
> CONFIG_DEBUG_PAGEALLOC=y
> CONFIG_DEBUG_PAGEALLOC_ENABLE_DEFAULT=y
> CONFIG_PAGE_POISONING=y
> CONFIG_DEBUG_OBJECTS=y
> CONFIG_DEBUG_OBJECTS_RCU_HEAD=y
> CONFIG_SCHED_STACK_END_CHECK=y
> CONFIG_KASAN=y
> CONFIG_KASAN_VMALLOC=y
> CONFIG_KASAN_EXTRA_INFO=y
> CONFIG_KFENCE=y
> # CONFIG_FTRACE is not set
> CONFIG_UNWINDER_FRAME_POINTER=y
> CONFIG_KUNIT=y
> CONFIG_TEST_DYNAMIC_DEBUG=m
>
Powered by blists - more mailing lists