| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CH0PR11MB5444A6196C5AC7D3AE960FD3E5C62@CH0PR11MB5444.namprd11.prod.outlook.com>
Date: Mon, 10 Jun 2024 20:42:50 +0000
From: "Cavitt, Jonathan" <jonathan.cavitt@...el.com>
To: "Vivi, Rodrigo" <rodrigo.vivi@...el.com>, "Souza, Jose"
<jose.souza@...el.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"intel-xe@...ts.freedesktop.org" <intel-xe@...ts.freedesktop.org>, "Mukesh
Ojha" <quic_mojha@...cinc.com>, Johannes Berg <johannes@...solutions.net>
Subject: RE: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()
-----Original Message-----
From: Vivi, Rodrigo <rodrigo.vivi@...el.com>
Sent: Monday, June 10, 2024 1:16 PM
To: Souza, Jose <jose.souza@...el.com>
Cc: linux-kernel@...r.kernel.org; intel-xe@...ts.freedesktop.org; Mukesh Ojha <quic_mojha@...cinc.com>; Johannes Berg <johannes@...solutions.net>; Cavitt, Jonathan <jonathan.cavitt@...el.com>
Subject: Re: [PATCH v5 1/2] devcoredump: Add dev_coredumpm_timeout()
>
> On Mon, Jun 10, 2024 at 09:11:32AM -0700, José Roberto de Souza wrote:
> > Add function to set a custom coredump timeout.
> >
> > For Xe driver usage, current 5 minutes timeout may be too short for
> > users to search and understand what needs to be done to capture
> > coredump to report bugs.
> >
> > We have plans to automate(distribute a udev script) it but at the end
> > will be up to distros and users to pack it so having a option to
> > increase the timeout is a safer option.
> >
> > v2:
> > - replace dev_coredump_timeout_set() by dev_coredumpm_timeout() (Mukesh)
> >
> > v3:
> > - make dev_coredumpm() static inline (Johannes)
> >
> > v5:
> > - rename DEVCOREDUMP_TIMEOUT -> DEVCD_TIMEOUT to avoid redefinition
> > in include/net/bluetooth/coredump.h
> >
> > Cc: Rodrigo Vivi <rodrigo.vivi@...el.com>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@...el.com>
>
> > Cc: Mukesh Ojha <quic_mojha@...cinc.com>
> > Cc: Johannes Berg <johannes@...solutions.net>
> > Cc: Jonathan Cavitt <jonathan.cavitt@...el.com>
> > Acked-by: Jonathan Cavitt <jonathan.cavitt@...el.com>
>
> Jonathan, also ack to merge this through drm-next flow?
Ack clear for drm-next flow. Actually, you can upgrade that to a
Reviewed-by: Jonathan Cavitt <jonathan.cavitt@...el.com>
I provided the Ack back before I knew I was allowed to provide
Reviewed-bys, or that they were different from Acks even in my
case. I trust that my past-self who originally reviewed this meant
for the Ack to be a stand-in as a non-committal RB.
-Jonathan Cavitt
>
> > Signed-off-by: José Roberto de Souza <jose.souza@...el.com>
> > ---
> > drivers/base/devcoredump.c | 23 ++++++++--------
> > include/linux/devcoredump.h | 54 ++++++++++++++++++++++++++++---------
> > 2 files changed, 54 insertions(+), 23 deletions(-)
> >
> > diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
> > index 82aeb09b3d1b5..c795edad1b969 100644
> > --- a/drivers/base/devcoredump.c
> > +++ b/drivers/base/devcoredump.c
> > @@ -18,9 +18,6 @@ static struct class devcd_class;
> > /* global disable flag, for security purposes */
> > static bool devcd_disabled;
> >
> > -/* if data isn't read by userspace after 5 minutes then delete it */
> > -#define DEVCD_TIMEOUT (HZ * 60 * 5)
> > -
> > struct devcd_entry {
> > struct device devcd_dev;
> > void *data;
> > @@ -328,7 +325,8 @@ void dev_coredump_put(struct device *dev)
> > EXPORT_SYMBOL_GPL(dev_coredump_put);
> >
> > /**
> > - * dev_coredumpm - create device coredump with read/free methods
> > + * dev_coredumpm_timeout - create device coredump with read/free methods with a
> > + * custom timeout.
> > * @dev: the struct device for the crashed device
> > * @owner: the module that contains the read/free functions, use %THIS_MODULE
> > * @data: data cookie for the @read/@...e functions
> > @@ -336,17 +334,20 @@ EXPORT_SYMBOL_GPL(dev_coredump_put);
> > * @gfp: allocation flags
> > * @read: function to read from the given buffer
> > * @free: function to free the given buffer
> > + * @timeout: time in jiffies to remove coredump
> > *
> > * Creates a new device coredump for the given device. If a previous one hasn't
> > * been read yet, the new coredump is discarded. The data lifetime is determined
> > * by the device coredump framework and when it is no longer needed the @free
> > * function will be called to free the data.
> > */
> > -void dev_coredumpm(struct device *dev, struct module *owner,
> > - void *data, size_t datalen, gfp_t gfp,
> > - ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > - void *data, size_t datalen),
> > - void (*free)(void *data))
> > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > + void *data, size_t datalen, gfp_t gfp,
> > + ssize_t (*read)(char *buffer, loff_t offset,
> > + size_t count, void *data,
> > + size_t datalen),
> > + void (*free)(void *data),
> > + unsigned long timeout)
> > {
> > static atomic_t devcd_count = ATOMIC_INIT(0);
> > struct devcd_entry *devcd;
> > @@ -403,7 +404,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
> > dev_set_uevent_suppress(&devcd->devcd_dev, false);
> > kobject_uevent(&devcd->devcd_dev.kobj, KOBJ_ADD);
> > INIT_DELAYED_WORK(&devcd->del_wk, devcd_del);
> > - schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
> > + schedule_delayed_work(&devcd->del_wk, timeout);
> > mutex_unlock(&devcd->mutex);
> > return;
> > put_device:
> > @@ -414,7 +415,7 @@ void dev_coredumpm(struct device *dev, struct module *owner,
> > free:
> > free(data);
> > }
> > -EXPORT_SYMBOL_GPL(dev_coredumpm);
> > +EXPORT_SYMBOL_GPL(dev_coredumpm_timeout);
> >
> > /**
> > * dev_coredumpsg - create device coredump that uses scatterlist as data
> > diff --git a/include/linux/devcoredump.h b/include/linux/devcoredump.h
> > index c8f7eb6cc1915..e3de1e545a4a5 100644
> > --- a/include/linux/devcoredump.h
> > +++ b/include/linux/devcoredump.h
> > @@ -12,6 +12,9 @@
> > #include <linux/scatterlist.h>
> > #include <linux/slab.h>
> >
> > +/* if data isn't read by userspace after 5 minutes then delete it */
> > +#define DEVCD_TIMEOUT (HZ * 60 * 5)
> > +
> > /*
> > * _devcd_free_sgtable - free all the memory of the given scatterlist table
> > * (i.e. both pages and scatterlist instances)
> > @@ -50,16 +53,17 @@ static inline void _devcd_free_sgtable(struct scatterlist *table)
> > kfree(delete_iter);
> > }
> >
> > -
> > #ifdef CONFIG_DEV_COREDUMP
> > void dev_coredumpv(struct device *dev, void *data, size_t datalen,
> > gfp_t gfp);
> >
> > -void dev_coredumpm(struct device *dev, struct module *owner,
> > - void *data, size_t datalen, gfp_t gfp,
> > - ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > - void *data, size_t datalen),
> > - void (*free)(void *data));
> > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > + void *data, size_t datalen, gfp_t gfp,
> > + ssize_t (*read)(char *buffer, loff_t offset,
> > + size_t count, void *data,
> > + size_t datalen),
> > + void (*free)(void *data),
> > + unsigned long timeout);
> >
> > void dev_coredumpsg(struct device *dev, struct scatterlist *table,
> > size_t datalen, gfp_t gfp);
> > @@ -72,12 +76,13 @@ static inline void dev_coredumpv(struct device *dev, void *data,
> > vfree(data);
> > }
> >
> > -static inline void
> > -dev_coredumpm(struct device *dev, struct module *owner,
> > - void *data, size_t datalen, gfp_t gfp,
> > - ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > - void *data, size_t datalen),
> > - void (*free)(void *data))
> > +void dev_coredumpm_timeout(struct device *dev, struct module *owner,
> > + void *data, size_t datalen, gfp_t gfp,
> > + ssize_t (*read)(char *buffer, loff_t offset,
> > + size_t count, void *data,
> > + size_t datalen),
> > + void (*free)(void *data),
> > + unsigned long timeout)
> > {
> > free(data);
> > }
> > @@ -92,4 +97,29 @@ static inline void dev_coredump_put(struct device *dev)
> > }
> > #endif /* CONFIG_DEV_COREDUMP */
> >
> > +/**
> > + * dev_coredumpm - create device coredump with read/free methods
> > + * @dev: the struct device for the crashed device
> > + * @owner: the module that contains the read/free functions, use %THIS_MODULE
> > + * @data: data cookie for the @read/@...e functions
> > + * @datalen: length of the data
> > + * @gfp: allocation flags
> > + * @read: function to read from the given buffer
> > + * @free: function to free the given buffer
> > + *
> > + * Creates a new device coredump for the given device. If a previous one hasn't
> > + * been read yet, the new coredump is discarded. The data lifetime is determined
> > + * by the device coredump framework and when it is no longer needed the @free
> > + * function will be called to free the data.
> > + */
> > +static inline void dev_coredumpm(struct device *dev, struct module *owner,
> > + void *data, size_t datalen, gfp_t gfp,
> > + ssize_t (*read)(char *buffer, loff_t offset, size_t count,
> > + void *data, size_t datalen),
> > + void (*free)(void *data))
> > +{
> > + dev_coredumpm_timeout(dev, owner, data, datalen, gfp, read, free,
> > + DEVCD_TIMEOUT);
> > +}
> > +
> > #endif /* __DEVCOREDUMP_H */
> > --
> > 2.45.2
> >
>
Powered by blists - more mailing lists