From: Michal Schmidt <mschmidt@redhat.com>
To: Rodolfo Giometti <giometti@enneenne.com>
Cc: "Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
"Christophe JAILLET" <christophe.jaillet@wanadoo.fr>,
"Dr. David Alan Gilbert" <linux@treblig.org>,
"Ma Ke" <make24@iscas.ac.cn>,
"Andrew Morton" <akpm@linux-foundation.org>,
"George Spelvin" <linux@horizon.com>,
linux-kernel@vger.kernel.org
Subject: [PATCH 5/6] pps: embed "dev" in the pps_device
Date: Mon, 2 Dec 2024 17:34:50 +0100 [thread overview]
Message-ID: <20241202163451.1442566-6-mschmidt@redhat.com> (raw)
In-Reply-To: <20241202163451.1442566-1-mschmidt@redhat.com>
Embed the struct device directly in struct pps_device.
Use device_initialize() + device_add() instead of device_create().
Advantages:
Avoids a separate allocation.
Lets us set dev as cdev's parent without the extra get_device() hack.
Signed-off-by: Michal Schmidt <mschmidt@redhat.com>
---
drivers/pps/clients/pps-gpio.c | 2 +-
drivers/pps/clients/pps-ldisc.c | 6 +--
drivers/pps/clients/pps_parport.c | 4 +-
drivers/pps/kapi.c | 10 ++---
drivers/pps/pps.c | 61 +++++++++++++++----------------
include/linux/pps_kernel.h | 2 +-
6 files changed, 42 insertions(+), 43 deletions(-)
diff --git a/drivers/pps/clients/pps-gpio.c b/drivers/pps/clients/pps-gpio.c
index 791fdc9326dd..a21bf56e7a87 100644
--- a/drivers/pps/clients/pps-gpio.c
+++ b/drivers/pps/clients/pps-gpio.c
@@ -214,7 +214,7 @@ static int pps_gpio_probe(struct platform_device *pdev)
return -EINVAL;
}
- dev_info(data->pps->dev, "Registered IRQ %d as PPS source\n",
+ dev_info(&data->pps->dev, "Registered IRQ %d as PPS source\n",
data->irq);
return 0;
diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldisc.c
index 443d6bae19d1..49ae33cca03d 100644
--- a/drivers/pps/clients/pps-ldisc.c
+++ b/drivers/pps/clients/pps-ldisc.c
@@ -32,7 +32,7 @@ static void pps_tty_dcd_change(struct tty_struct *tty, bool active)
pps_event(pps, &ts, active ? PPS_CAPTUREASSERT :
PPS_CAPTURECLEAR, NULL);
- dev_dbg(pps->dev, "PPS %s at %lu\n",
+ dev_dbg(&pps->dev, "PPS %s at %lu\n",
active ? "assert" : "clear", jiffies);
}
@@ -69,7 +69,7 @@ static int pps_tty_open(struct tty_struct *tty)
goto err_unregister;
}
- dev_info(pps->dev, "source \"%s\" added\n", info.path);
+ dev_info(&pps->dev, "source \"%s\" added\n", info.path);
return 0;
@@ -89,7 +89,7 @@ static void pps_tty_close(struct tty_struct *tty)
if (WARN_ON(!pps))
return;
- dev_info(pps->dev, "removed\n");
+ dev_info(&pps->dev, "removed\n");
pps_unregister_source(pps);
}
diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_parport.c
index abaffb4e1c1c..24db06750297 100644
--- a/drivers/pps/clients/pps_parport.c
+++ b/drivers/pps/clients/pps_parport.c
@@ -81,7 +81,7 @@ static void parport_irq(void *handle)
/* check the signal (no signal means the pulse is lost this time) */
if (!signal_is_set(port)) {
local_irq_restore(flags);
- dev_err(dev->pps->dev, "lost the signal\n");
+ dev_err(&dev->pps->dev, "lost the signal\n");
goto out_assert;
}
@@ -98,7 +98,7 @@ static void parport_irq(void *handle)
/* timeout */
dev->cw_err++;
if (dev->cw_err >= CLEAR_WAIT_MAX_ERRORS) {
- dev_err(dev->pps->dev, "disabled clear edge capture after %d"
+ dev_err(&dev->pps->dev, "disabled clear edge capture after %d"
" timeouts\n", dev->cw_err);
dev->cw = 0;
dev->cw_err = 0;
diff --git a/drivers/pps/kapi.c b/drivers/pps/kapi.c
index d9d566f70ed1..a76685c147ee 100644
--- a/drivers/pps/kapi.c
+++ b/drivers/pps/kapi.c
@@ -41,7 +41,7 @@ static void pps_add_offset(struct pps_ktime *ts, struct pps_ktime *offset)
static void pps_echo_client_default(struct pps_device *pps, int event,
void *data)
{
- dev_info(pps->dev, "echo %s %s\n",
+ dev_info(&pps->dev, "echo %s %s\n",
event & PPS_CAPTUREASSERT ? "assert" : "",
event & PPS_CAPTURECLEAR ? "clear" : "");
}
@@ -112,7 +112,7 @@ struct pps_device *pps_register_source(struct pps_source_info *info,
goto kfree_pps;
}
- dev_info(pps->dev, "new PPS source %s\n", info->name);
+ dev_info(&pps->dev, "new PPS source %s\n", info->name);
return pps;
@@ -166,7 +166,7 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
/* check event type */
BUG_ON((event & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR)) == 0);
- dev_dbg(pps->dev, "PPS event at %lld.%09ld\n",
+ dev_dbg(&pps->dev, "PPS event at %lld.%09ld\n",
(s64)ts->ts_real.tv_sec, ts->ts_real.tv_nsec);
timespec_to_pps_ktime(&ts_real, ts->ts_real);
@@ -188,7 +188,7 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
/* Save the time stamp */
pps->assert_tu = ts_real;
pps->assert_sequence++;
- dev_dbg(pps->dev, "capture assert seq #%u\n",
+ dev_dbg(&pps->dev, "capture assert seq #%u\n",
pps->assert_sequence);
captured = ~0;
@@ -202,7 +202,7 @@ void pps_event(struct pps_device *pps, struct pps_event_time *ts, int event,
/* Save the time stamp */
pps->clear_tu = ts_real;
pps->clear_sequence++;
- dev_dbg(pps->dev, "capture clear seq #%u\n",
+ dev_dbg(&pps->dev, "capture clear seq #%u\n",
pps->clear_sequence);
captured = ~0;
diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index f90ee483d343..64a8b34aa4ce 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -62,7 +62,7 @@ static int pps_cdev_pps_fetch(struct pps_device *pps, struct pps_fdata *fdata)
else {
unsigned long ticks;
- dev_dbg(pps->dev, "timeout %lld.%09d\n",
+ dev_dbg(&pps->dev, "timeout %lld.%09d\n",
(long long) fdata->timeout.sec,
fdata->timeout.nsec);
ticks = fdata->timeout.sec * HZ;
@@ -80,7 +80,7 @@ static int pps_cdev_pps_fetch(struct pps_device *pps, struct pps_fdata *fdata)
/* Check for pending signals */
if (err == -ERESTARTSYS) {
- dev_dbg(pps->dev, "pending signal caught\n");
+ dev_dbg(&pps->dev, "pending signal caught\n");
return -EINTR;
}
@@ -98,7 +98,7 @@ static long pps_cdev_ioctl(struct file *file,
switch (cmd) {
case PPS_GETPARAMS:
- dev_dbg(pps->dev, "PPS_GETPARAMS\n");
+ dev_dbg(&pps->dev, "PPS_GETPARAMS\n");
spin_lock_irq(&pps->lock);
@@ -114,7 +114,7 @@ static long pps_cdev_ioctl(struct file *file,
break;
case PPS_SETPARAMS:
- dev_dbg(pps->dev, "PPS_SETPARAMS\n");
+ dev_dbg(&pps->dev, "PPS_SETPARAMS\n");
/* Check the capabilities */
if (!capable(CAP_SYS_TIME))
@@ -124,14 +124,14 @@ static long pps_cdev_ioctl(struct file *file,
if (err)
return -EFAULT;
if (!(params.mode & (PPS_CAPTUREASSERT | PPS_CAPTURECLEAR))) {
- dev_dbg(pps->dev, "capture mode unspecified (%x)\n",
+ dev_dbg(&pps->dev, "capture mode unspecified (%x)\n",
params.mode);
return -EINVAL;
}
/* Check for supported capabilities */
if ((params.mode & ~pps->info.mode) != 0) {
- dev_dbg(pps->dev, "unsupported capabilities (%x)\n",
+ dev_dbg(&pps->dev, "unsupported capabilities (%x)\n",
params.mode);
return -EINVAL;
}
@@ -144,7 +144,7 @@ static long pps_cdev_ioctl(struct file *file,
/* Restore the read only parameters */
if ((params.mode & (PPS_TSFMT_TSPEC | PPS_TSFMT_NTPFP)) == 0) {
/* section 3.3 of RFC 2783 interpreted */
- dev_dbg(pps->dev, "time format unspecified (%x)\n",
+ dev_dbg(&pps->dev, "time format unspecified (%x)\n",
params.mode);
pps->params.mode |= PPS_TSFMT_TSPEC;
}
@@ -165,7 +165,7 @@ static long pps_cdev_ioctl(struct file *file,
break;
case PPS_GETCAP:
- dev_dbg(pps->dev, "PPS_GETCAP\n");
+ dev_dbg(&pps->dev, "PPS_GETCAP\n");
err = put_user(pps->info.mode, iuarg);
if (err)
@@ -176,7 +176,7 @@ static long pps_cdev_ioctl(struct file *file,
case PPS_FETCH: {
struct pps_fdata fdata;
- dev_dbg(pps->dev, "PPS_FETCH\n");
+ dev_dbg(&pps->dev, "PPS_FETCH\n");
err = copy_from_user(&fdata, uarg, sizeof(struct pps_fdata));
if (err)
@@ -206,7 +206,7 @@ static long pps_cdev_ioctl(struct file *file,
case PPS_KC_BIND: {
struct pps_bind_args bind_args;
- dev_dbg(pps->dev, "PPS_KC_BIND\n");
+ dev_dbg(&pps->dev, "PPS_KC_BIND\n");
/* Check the capabilities */
if (!capable(CAP_SYS_TIME))
@@ -218,7 +218,7 @@ static long pps_cdev_ioctl(struct file *file,
/* Check for supported capabilities */
if ((bind_args.edge & ~pps->info.mode) != 0) {
- dev_err(pps->dev, "unsupported capabilities (%x)\n",
+ dev_err(&pps->dev, "unsupported capabilities (%x)\n",
bind_args.edge);
return -EINVAL;
}
@@ -227,7 +227,7 @@ static long pps_cdev_ioctl(struct file *file,
if (bind_args.tsformat != PPS_TSFMT_TSPEC ||
(bind_args.edge & ~PPS_CAPTUREBOTH) != 0 ||
bind_args.consumer != PPS_KC_HARDPPS) {
- dev_err(pps->dev, "invalid kernel consumer bind"
+ dev_err(&pps->dev, "invalid kernel consumer bind"
" parameters (%x)\n", bind_args.edge);
return -EINVAL;
}
@@ -259,7 +259,7 @@ static long pps_cdev_compat_ioctl(struct file *file,
struct pps_fdata fdata;
int err;
- dev_dbg(pps->dev, "PPS_FETCH\n");
+ dev_dbg(&pps->dev, "PPS_FETCH\n");
err = copy_from_user(&compat, uarg, sizeof(struct pps_fdata_compat));
if (err)
@@ -319,14 +319,13 @@ static const struct file_operations pps_cdev_fops = {
static void pps_device_destruct(struct device *dev)
{
- struct pps_device *pps = dev_get_drvdata(dev);
+ struct pps_device *pps = container_of(dev, struct pps_device, dev);
/* Now we can release the ID for re-use */
pr_debug("deallocating pps%d\n", pps->id);
scoped_guard(mutex, &pps_idr_lock)
idr_remove(&pps_idr, pps->id);
- kfree(dev);
kfree(pps);
}
@@ -353,26 +352,25 @@ int pps_register_cdev(struct pps_device *pps)
devt = MKDEV(MAJOR(pps_devt), pps->id);
+ device_initialize(&pps->dev);
+ pps->dev.devt = devt;
+ pps->dev.class = pps_class;
+ pps->dev.parent = pps->info.dev;
+ err = dev_set_name(&pps->dev, "pps%d", pps->id);
+ if (err)
+ goto put_dev;
cdev_init(&pps->cdev, &pps_cdev_fops);
pps->cdev.owner = pps->info.owner;
-
+ cdev_set_parent(&pps->cdev, &pps->dev.kobj);
err = cdev_add(&pps->cdev, devt, 1);
if (err)
- goto free_idr;
-
- pps->dev = device_create(pps_class, pps->info.dev, devt, pps,
- "pps%d", pps->id);
- if (IS_ERR(pps->dev)) {
- err = PTR_ERR(pps->dev);
+ goto put_dev;
+ err = device_add(&pps->dev);
+ if (err)
goto del_cdev;
- }
-
- cdev_set_parent(&pps->cdev, &pps->dev->kobj);
- /* Compensate for setting the parent after cdev_add() */
- get_device(pps->dev);
/* Override the release function with our own */
- pps->dev->release = pps_device_destruct;
+ pps->dev.release = pps_device_destruct;
pr_debug("source %s got cdev (%d:%d)\n", pps->info.name,
MAJOR(pps_devt), pps->id);
@@ -381,8 +379,8 @@ int pps_register_cdev(struct pps_device *pps)
del_cdev:
cdev_del(&pps->cdev);
-
-free_idr:
+put_dev:
+ put_device(&pps->dev);
scoped_guard(mutex, &pps_idr_lock)
idr_remove(&pps_idr, pps->id);
pr_err("%s: failed to add char device %d:%d\n",
@@ -394,8 +392,9 @@ void pps_unregister_cdev(struct pps_device *pps)
{
pr_debug("unregistering pps%d\n", pps->id);
pps->lookup_cookie = NULL;
- device_destroy(pps_class, pps->dev->devt);
+ device_del(&pps->dev);
cdev_del(&pps->cdev);
+ put_device(&pps->dev);
}
/*
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index 78c8ac4951b5..d1edacb7e63e 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -57,7 +57,7 @@ struct pps_device {
unsigned int id; /* PPS source unique ID */
void const *lookup_cookie; /* For pps_lookup_dev() only */
struct cdev cdev;
- struct device *dev;
+ struct device dev;
struct fasync_struct *async_queue; /* fasync method */
spinlock_t lock;
};
--
2.47.0
next prev parent reply other threads:[~2024-12-02 16:35 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-02 16:34 [PATCH 0/6] pps: fix a UAF and clean up code Michal Schmidt
2024-12-02 16:34 ` [PATCH 1/6] pps: fix cdev use-after-free Michal Schmidt
2024-12-02 16:34 ` [PATCH 2/6] pps: simplify pps_idr_lock locking Michal Schmidt
2024-12-02 16:34 ` [PATCH 3/6] pps: use scoped_guard for pps_idr_lock Michal Schmidt
2024-12-03 9:06 ` Uwe Kleine-König
2024-12-02 16:34 ` [PATCH 4/6] pps: print error in both cdev and dev error paths in pps_register_cdev() Michal Schmidt
2024-12-02 16:34 ` Michal Schmidt [this message]
2024-12-03 1:02 ` [PATCH 5/6] pps: embed "dev" in the pps_device kernel test robot
2024-12-03 2:04 ` kernel test robot
2024-12-02 16:34 ` [PATCH 6/6] pps: use cdev_device_add() Michal Schmidt
2024-12-02 16:56 ` [PATCH 0/6] pps: fix a UAF and clean up code Calvin Owens
2024-12-02 17:58 ` Michal Schmidt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241202163451.1442566-6-mschmidt@redhat.com \
--to=mschmidt@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=christophe.jaillet@wanadoo.fr \
--cc=giometti@enneenne.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@horizon.com \
--cc=linux@treblig.org \
--cc=make24@iscas.ac.cn \
--cc=u.kleine-koenig@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.