From nobody Sun Nov 24 14:56:54 2024 Received: from mail-pf1-f172.google.com (mail-pf1-f172.google.com [209.85.210.172]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 90D44282F1 for ; Tue, 5 Nov 2024 07:56:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.210.172 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730793412; cv=none; b=tWnuwcwKkwGnDARzW7fxwsBtABhSxIzvzdb0Hiap4WSzemvaXzRBXVXztEXvmWtuS0b2w7qQ1q7mfSQgXGXxmNWmLa0VvVRFe2ydcpfhuvurNBfBx+x1yIdm7V1qgZPeU62pgFSjohk8k2LsDjVHTfLewV42MPS1tpEi845i3kM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1730793412; c=relaxed/simple; bh=bq6MSN59iN0IVMJ9aNuudKrXZcyiK42iS5O8Dh844qc=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=FMA3wcjSa5Te10ZfyIHB2grxpo3ouGUZVhgAUYH+Sf7wHY5m/JmoFXkLNrlXPRUcKBjo1WmnkeSOGuHi5PuOgnwKSNuJU9vNhPJiLClsXFf/6UoobWhasx4oIy/rD+BTqxPKLDpk+Ro3z9NNrxYyiclxF74oKSgc1Gzf29PJyVY= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=wbinvd.org; spf=pass smtp.mailfrom=wbinvd.org; dkim=pass (2048-bit key) header.d=wbinvd.org header.i=@wbinvd.org header.b=e/KOvMxD; arc=none smtp.client-ip=209.85.210.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=wbinvd.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=wbinvd.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=wbinvd.org header.i=@wbinvd.org header.b="e/KOvMxD" Received: by mail-pf1-f172.google.com with SMTP id d2e1a72fcca58-720e94d36c8so1978902b3a.1 for ; Mon, 04 Nov 2024 23:56:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=wbinvd.org; s=wbinvd; t=1730793410; x=1731398210; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=xdJSbaBNt8yK/8bznTA9Mve2MmveJb2TotpjT1i1g0o=; b=e/KOvMxDhdTRpP9SYdUnbx9P+7tlzAvnjVuErLNg5l8olX+rsAZMlVlxw2bVR0Zciz kyCeuxEhkkoOcdia92xQz5M2XL0p+AEAJRNH1MxSSoImnqVxI39pI+TEdngGYjL04XA5 /eS2/aiy4KaczW4lWMdScG6jFQTpN6W6v6x6YvAu7jZ2A+b0wihOtIKvJ1/C43nJ4IyX mZmsmXDkTeLcsrIEXN2ofGJsHwnbWfF51eM5p7PmlODM2nVhpT0tElaL+9Y07MQXDlNO 9NRKwTav7qG7PH6SwyAFlhsOsxfSySEwkoc9zvoZ3c00u+y0sOX1vutChtcohjEnVb5m WgDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730793410; x=1731398210; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=xdJSbaBNt8yK/8bznTA9Mve2MmveJb2TotpjT1i1g0o=; b=hugfdY32d/85OzQyOi4ALq85aXSmLLa3YcOhj6JL6WZ//4v1D3Niq1D5MmOMHNVjCe +I97TggFeroOH7A6lIe9mu+tGq+ct1h27KYMVBxmAOELnT0gzv5iI2Rgi2kTERZNNFWP 6rDafL7s5Q0WslCsbBPwzkGK1f/LlFpEelcfSycbfQ7PjfkpUGsdt0abP2eSmxEYpNYI sSg9/YkY5gjR5z5K12fVdQsEqI97tB9q0lEHuhnA6P0Zle1qBFUa2zNTdWDMkN03/AYh 00yt3fcE/Q/HObY9KOabtlw0iifP4ScuxIeMmkZbax5CL+UggH2E/BoRgQ9/idRiHv+P af3A== X-Forwarded-Encrypted: i=1; AJvYcCVnXm4zuza4t4WQTRblD9PaZc1I5fLcyK/Nw5julbmAmrRFdP9K5Vu6kswt/ws9Lyq2Rvj/CIHSB/513f0=@vger.kernel.org X-Gm-Message-State: AOJu0YxQi6vbMxVrGlvxmFoHSd+X4ltEFFgwqq9/iKTREgtLx1BIMiaG tych651mxIWXhB+8zDPPokFGu7jnZekPAM8vrzXiDjd5RQZRvRIJqBcxjhVx6OXg99wWBWxaML1 m X-Google-Smtp-Source: AGHT+IHQ9wKAB/aC/AFfcDiiWZhQDmy9WMlE7EZV8znIMIf1XqwGFSPcWDwqiUdRS+k6BvhIQGI3Pg== X-Received: by 2002:aa7:93c7:0:b0:71e:7258:c69b with SMTP id d2e1a72fcca58-720c98495d5mr20957892b3a.14.1730793409654; Mon, 04 Nov 2024 23:56:49 -0800 (PST) Received: from mozart.vkv.me (192-184-160-110.fiber.dynamic.sonic.net. [192.184.160.110]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-720bc313e0csm8967641b3a.189.2024.11.04.23.56.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 Nov 2024 23:56:49 -0800 (PST) From: Calvin Owens To: Greg Kroah-Hartman Cc: Rodolfo Giometti , George Spelvin , linux-kernel@vger.kernel.org, stable@vger.kernel.org Subject: [PATCH v3] pps: Fix a use-after-free Date: Mon, 4 Nov 2024 23:56:19 -0800 Message-ID: X-Mailer: git-send-email 2.45.2 In-Reply-To: <2024101350-jinx-haggler-5aca@gregkh> References: <2024101350-jinx-haggler-5aca@gregkh> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" On a board running ntpd and gpsd, I'm seeing a consistent use-after-free in sys_exit() from gpsd when rebooting: pps pps1: removed ------------[ cut here ]------------ kobject: '(null)' (00000000db4bec24): is not initialized, yet kobject_p= ut() is being called. WARNING: CPU: 2 PID: 440 at lib/kobject.c:734 kobject_put+0x120/0x150 CPU: 2 UID: 299 PID: 440 Comm: gpsd Not tainted 6.11.0-rc6-00308-gb31c4= 4928842 #1 Hardware name: Raspberry Pi 4 Model B Rev 1.1 (DT) pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=3D--) pc : kobject_put+0x120/0x150 lr : kobject_put+0x120/0x150 sp : ffffffc0803d3ae0 x29: ffffffc0803d3ae0 x28: ffffff8042dc9738 x27: 0000000000000001 x26: 0000000000000000 x25: ffffff8042dc9040 x24: ffffff8042dc9440 x23: ffffff80402a4620 x22: ffffff8042ef4bd0 x21: ffffff80405cb600 x20: 000000000008001b x19: ffffff8040b3b6e0 x18: 0000000000000000 x17: 0000000000000000 x16: 0000000000000000 x15: 696e6920746f6e20 x14: 7369203a29343263 x13: 205d303434542020 x12: 0000000000000000 x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000 x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000 x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000000 x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000000 Call trace: kobject_put+0x120/0x150 cdev_put+0x20/0x3c __fput+0x2c4/0x2d8 ____fput+0x1c/0x38 task_work_run+0x70/0xfc do_exit+0x2a0/0x924 do_group_exit+0x34/0x90 get_signal+0x7fc/0x8c0 do_signal+0x128/0x13b4 do_notify_resume+0xdc/0x160 el0_svc+0xd4/0xf8 el0t_64_sync_handler+0x140/0x14c el0t_64_sync+0x190/0x194 ---[ end trace 0000000000000000 ]--- ...followed by more symptoms of corruption, with similar stacks: refcount_t: underflow; use-after-free. kernel BUG at lib/list_debug.c:62! Kernel panic - not syncing: Oops - BUG: Fatal exception This happens because pps_device_destruct() frees the pps_device with the embedded cdev immediately after calling cdev_del(), but, as the comment above cdev_del() notes, fops for previously opened cdevs are still callable even after cdev_del() returns. I think this bug has always been there: I can't explain why it suddenly started happening every time I reboot this particular board. In commit d953e0e837e6 ("pps: Fix a use-after free bug when unregistering a source."), George Spelvin suggested removing the embedded cdev. That seems like the simplest way to fix this, so I've implemented his suggestion, using __register_chrdev() with pps_idr becoming the source of truth for which minor corresponds to which device. But now that pps_idr defines userspace visibility instead of cdev_add(), we need to be sure the pps->dev refcount can't reach zero while userspace can still find it again. So, the idr_remove() call moves to pps_unregister_cdev(), and pps_idr now holds a reference to pps->dev. pps_core: source serial1 got cdev (251:1) <...> pps pps1: removed pps_core: unregistering pps1 pps_core: deallocating pps1 Fixes: d953e0e837e6 ("pps: Fix a use-after free bug when unregistering a so= urce.") Cc: stable@vger.kernel.org Signed-off-by: Calvin Owens --- Changes in v3: - Shorten patch title - Embed the device struct in the pps struct - Use foo_device(&pps->dev) instead of kobject_foo(&pps->dev.kobj) Changes in v2: - Don't move pr_debug() from pps_device_destruct() to pps_unregister_cdev() - Actually add stable@vger.kernel.org to CC --- drivers/pps/clients/pps-gpio.c | 2 +- drivers/pps/clients/pps-ktimer.c | 4 +- drivers/pps/clients/pps-ldisc.c | 6 +- drivers/pps/clients/pps_parport.c | 4 +- drivers/pps/kapi.c | 10 +-- drivers/pps/kc.c | 10 +-- drivers/pps/pps.c | 127 ++++++++++++++++-------------- include/linux/pps_kernel.h | 3 +- 8 files changed, 85 insertions(+), 81 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; } =20 - 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); =20 return 0; diff --git a/drivers/pps/clients/pps-ktimer.c b/drivers/pps/clients/pps-kti= mer.c index d33106bd7a29..00cf406377a1 100644 --- a/drivers/pps/clients/pps-ktimer.c +++ b/drivers/pps/clients/pps-ktimer.c @@ -56,7 +56,7 @@ static struct pps_source_info pps_ktimer_info =3D { =20 static void __exit pps_ktimer_exit(void) { - dev_info(pps->dev, "ktimer PPS source unregistered\n"); + dev_info(&pps->dev, "ktimer PPS source unregistered\n"); =20 del_timer_sync(&ktimer); pps_unregister_source(pps); @@ -74,7 +74,7 @@ static int __init pps_ktimer_init(void) timer_setup(&ktimer, pps_ktimer_event, 0); mod_timer(&ktimer, jiffies + HZ); =20 - dev_info(pps->dev, "ktimer PPS source registered\n"); + dev_info(&pps->dev, "ktimer PPS source registered\n"); =20 return 0; } diff --git a/drivers/pps/clients/pps-ldisc.c b/drivers/pps/clients/pps-ldis= c.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, bo= ol active) pps_event(pps, &ts, active ? PPS_CAPTUREASSERT : PPS_CAPTURECLEAR, NULL); =20 - dev_dbg(pps->dev, "PPS %s at %lu\n", + dev_dbg(&pps->dev, "PPS %s at %lu\n", active ? "assert" : "clear", jiffies); } =20 @@ -69,7 +69,7 @@ static int pps_tty_open(struct tty_struct *tty) goto err_unregister; } =20 - dev_info(pps->dev, "source \"%s\" added\n", info.path); + dev_info(&pps->dev, "source \"%s\" added\n", info.path); =20 return 0; =20 @@ -89,7 +89,7 @@ static void pps_tty_close(struct tty_struct *tty) if (WARN_ON(!pps)) return; =20 - dev_info(pps->dev, "removed\n"); + dev_info(&pps->dev, "removed\n"); pps_unregister_source(pps); } =20 diff --git a/drivers/pps/clients/pps_parport.c b/drivers/pps/clients/pps_pa= rport.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; } =20 @@ -98,7 +98,7 @@ static void parport_irq(void *handle) /* timeout */ dev->cw_err++; if (dev->cw_err >=3D 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 =3D 0; dev->cw_err =3D 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 p= ps_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_sourc= e_info *info, goto kfree_pps; } =20 - dev_info(pps->dev, "new PPS source %s\n", info->name); + dev_info(&pps->dev, "new PPS source %s\n", info->name); =20 return pps; =20 @@ -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)) =3D=3D 0); =20 - 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); =20 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 =3D 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); =20 captured =3D ~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 =3D 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); =20 captured =3D ~0; diff --git a/drivers/pps/kc.c b/drivers/pps/kc.c index 50dc59af45be..fbd23295afd7 100644 --- a/drivers/pps/kc.c +++ b/drivers/pps/kc.c @@ -43,11 +43,11 @@ int pps_kc_bind(struct pps_device *pps, struct pps_bind= _args *bind_args) pps_kc_hardpps_mode =3D 0; pps_kc_hardpps_dev =3D NULL; spin_unlock_irq(&pps_kc_hardpps_lock); - dev_info(pps->dev, "unbound kernel" + dev_info(&pps->dev, "unbound kernel" " consumer\n"); } else { spin_unlock_irq(&pps_kc_hardpps_lock); - dev_err(pps->dev, "selected kernel consumer" + dev_err(&pps->dev, "selected kernel consumer" " is not bound\n"); return -EINVAL; } @@ -57,11 +57,11 @@ int pps_kc_bind(struct pps_device *pps, struct pps_bind= _args *bind_args) pps_kc_hardpps_mode =3D bind_args->edge; pps_kc_hardpps_dev =3D pps; spin_unlock_irq(&pps_kc_hardpps_lock); - dev_info(pps->dev, "bound kernel consumer: " + dev_info(&pps->dev, "bound kernel consumer: " "edge=3D0x%x\n", bind_args->edge); } else { spin_unlock_irq(&pps_kc_hardpps_lock); - dev_err(pps->dev, "another kernel consumer" + dev_err(&pps->dev, "another kernel consumer" " is already bound\n"); return -EINVAL; } @@ -83,7 +83,7 @@ void pps_kc_remove(struct pps_device *pps) pps_kc_hardpps_mode =3D 0; pps_kc_hardpps_dev =3D NULL; spin_unlock_irq(&pps_kc_hardpps_lock); - dev_info(pps->dev, "unbound kernel consumer" + dev_info(&pps->dev, "unbound kernel consumer" " on device removal\n"); } else spin_unlock_irq(&pps_kc_hardpps_lock); diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c index 25d47907db17..6a02245ea35f 100644 --- a/drivers/pps/pps.c +++ b/drivers/pps/pps.c @@ -25,7 +25,7 @@ * Local variables */ =20 -static dev_t pps_devt; +static int pps_major; static struct class *pps_class; =20 static DEFINE_MUTEX(pps_idr_lock); @@ -62,7 +62,7 @@ static int pps_cdev_pps_fetch(struct pps_device *pps, str= uct pps_fdata *fdata) else { unsigned long ticks; =20 - 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 =3D fdata->timeout.sec * HZ; @@ -80,7 +80,7 @@ static int pps_cdev_pps_fetch(struct pps_device *pps, str= uct pps_fdata *fdata) =20 /* Check for pending signals */ if (err =3D=3D -ERESTARTSYS) { - dev_dbg(pps->dev, "pending signal caught\n"); + dev_dbg(&pps->dev, "pending signal caught\n"); return -EINTR; } =20 @@ -98,7 +98,7 @@ static long pps_cdev_ioctl(struct file *file, =20 switch (cmd) { case PPS_GETPARAMS: - dev_dbg(pps->dev, "PPS_GETPARAMS\n"); + dev_dbg(&pps->dev, "PPS_GETPARAMS\n"); =20 spin_lock_irq(&pps->lock); =20 @@ -114,7 +114,7 @@ static long pps_cdev_ioctl(struct file *file, break; =20 case PPS_SETPARAMS: - dev_dbg(pps->dev, "PPS_SETPARAMS\n"); + dev_dbg(&pps->dev, "PPS_SETPARAMS\n"); =20 /* 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; } =20 /* Check for supported capabilities */ if ((params.mode & ~pps->info.mode) !=3D 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)) =3D=3D 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 |=3D PPS_TSFMT_TSPEC; } @@ -165,7 +165,7 @@ static long pps_cdev_ioctl(struct file *file, break; =20 case PPS_GETCAP: - dev_dbg(pps->dev, "PPS_GETCAP\n"); + dev_dbg(&pps->dev, "PPS_GETCAP\n"); =20 err =3D 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; =20 - dev_dbg(pps->dev, "PPS_FETCH\n"); + dev_dbg(&pps->dev, "PPS_FETCH\n"); =20 err =3D 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; =20 - dev_dbg(pps->dev, "PPS_KC_BIND\n"); + dev_dbg(&pps->dev, "PPS_KC_BIND\n"); =20 /* Check the capabilities */ if (!capable(CAP_SYS_TIME)) @@ -218,7 +218,7 @@ static long pps_cdev_ioctl(struct file *file, =20 /* Check for supported capabilities */ if ((bind_args.edge & ~pps->info.mode) !=3D 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 !=3D PPS_TSFMT_TSPEC || (bind_args.edge & ~PPS_CAPTUREBOTH) !=3D 0 || bind_args.consumer !=3D 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; =20 - dev_dbg(pps->dev, "PPS_FETCH\n"); + dev_dbg(&pps->dev, "PPS_FETCH\n"); =20 err =3D copy_from_user(&compat, uarg, sizeof(struct pps_fdata_compat)); if (err) @@ -296,20 +296,36 @@ static long pps_cdev_compat_ioctl(struct file *file, #define pps_cdev_compat_ioctl NULL #endif =20 +static struct pps_device *pps_idr_get(unsigned long id) +{ + struct pps_device *pps; + + mutex_lock(&pps_idr_lock); + pps =3D idr_find(&pps_idr, id); + if (pps) + get_device(&pps->dev); + + mutex_unlock(&pps_idr_lock); + return pps; +} + static int pps_cdev_open(struct inode *inode, struct file *file) { - struct pps_device *pps =3D container_of(inode->i_cdev, - struct pps_device, cdev); + struct pps_device *pps =3D pps_idr_get(iminor(inode)); + + if (!pps) + return -ENODEV; + file->private_data =3D pps; - kobject_get(&pps->dev->kobj); return 0; } =20 static int pps_cdev_release(struct inode *inode, struct file *file) { - struct pps_device *pps =3D container_of(inode->i_cdev, - struct pps_device, cdev); - kobject_put(&pps->dev->kobj); + struct pps_device *pps =3D file->private_data; + + WARN_ON(pps->id !=3D iminor(inode)); + put_device(&pps->dev); return 0; } =20 @@ -331,22 +347,13 @@ static void pps_device_destruct(struct device *dev) { struct pps_device *pps =3D dev_get_drvdata(dev); =20 - cdev_del(&pps->cdev); - - /* Now we can release the ID for re-use */ pr_debug("deallocating pps%d\n", pps->id); - mutex_lock(&pps_idr_lock); - idr_remove(&pps_idr, pps->id); - mutex_unlock(&pps_idr_lock); - - kfree(dev); kfree(pps); } =20 int pps_register_cdev(struct pps_device *pps) { int err; - dev_t devt; =20 mutex_lock(&pps_idr_lock); /* @@ -363,40 +370,29 @@ int pps_register_cdev(struct pps_device *pps) goto out_unlock; } pps->id =3D err; - mutex_unlock(&pps_idr_lock); - - devt =3D MKDEV(MAJOR(pps_devt), pps->id); - - cdev_init(&pps->cdev, &pps_cdev_fops); - pps->cdev.owner =3D pps->info.owner; =20 - err =3D cdev_add(&pps->cdev, devt, 1); - if (err) { - pr_err("%s: failed to add char device %d:%d\n", - pps->info.name, MAJOR(pps_devt), pps->id); + pps->dev.class =3D pps_class; + pps->dev.parent =3D pps->info.dev; + pps->dev.devt =3D MKDEV(pps_major, pps->id); + dev_set_drvdata(&pps->dev, pps); + dev_set_name(&pps->dev, "pps%d", pps->id); + err =3D device_register(&pps->dev); + if (err) goto free_idr; - } - pps->dev =3D device_create(pps_class, pps->info.dev, devt, pps, - "pps%d", pps->id); - if (IS_ERR(pps->dev)) { - err =3D PTR_ERR(pps->dev); - goto del_cdev; - } =20 /* Override the release function with our own */ - pps->dev->release =3D pps_device_destruct; + pps->dev.release =3D pps_device_destruct; =20 - pr_debug("source %s got cdev (%d:%d)\n", pps->info.name, - MAJOR(pps_devt), pps->id); + pr_debug("source %s got cdev (%d:%d)\n", pps->info.name, pps_major, + pps->id); =20 + get_device(&pps->dev); + mutex_unlock(&pps_idr_lock); return 0; =20 -del_cdev: - cdev_del(&pps->cdev); - free_idr: - mutex_lock(&pps_idr_lock); idr_remove(&pps_idr, pps->id); + put_device(&pps->dev); out_unlock: mutex_unlock(&pps_idr_lock); return err; @@ -406,7 +402,13 @@ void pps_unregister_cdev(struct pps_device *pps) { pr_debug("unregistering pps%d\n", pps->id); pps->lookup_cookie =3D NULL; - device_destroy(pps_class, pps->dev->devt); + device_destroy(pps_class, pps->dev.devt); + + /* Now we can release the ID for re-use */ + mutex_lock(&pps_idr_lock); + idr_remove(&pps_idr, pps->id); + put_device(&pps->dev); + mutex_unlock(&pps_idr_lock); } =20 /* @@ -426,6 +428,11 @@ void pps_unregister_cdev(struct pps_device *pps) * so that it will not be used again, even if the pps device cannot * be removed from the idr due to pending references holding the minor * number in use. + * + * Since pps_idr holds a reference to the device, the returned + * pps_device is guaranteed to be valid until pps_unregister_cdev() is + * called on it. But after calling pps_unregister_cdev(), it may be + * freed at any time. */ struct pps_device *pps_lookup_dev(void const *cookie) { @@ -448,13 +455,11 @@ EXPORT_SYMBOL(pps_lookup_dev); static void __exit pps_exit(void) { class_destroy(pps_class); - unregister_chrdev_region(pps_devt, PPS_MAX_SOURCES); + __unregister_chrdev(pps_major, 0, PPS_MAX_SOURCES, "pps"); } =20 static int __init pps_init(void) { - int err; - pps_class =3D class_create("pps"); if (IS_ERR(pps_class)) { pr_err("failed to allocate class\n"); @@ -462,8 +467,9 @@ static int __init pps_init(void) } pps_class->dev_groups =3D pps_groups; =20 - err =3D alloc_chrdev_region(&pps_devt, 0, PPS_MAX_SOURCES, "pps"); - if (err < 0) { + pps_major =3D __register_chrdev(0, 0, PPS_MAX_SOURCES, "pps", + &pps_cdev_fops); + if (pps_major < 0) { pr_err("failed to allocate char device region\n"); goto remove_class; } @@ -476,8 +482,7 @@ static int __init pps_init(void) =20 remove_class: class_destroy(pps_class); - - return err; + return pps_major; } =20 subsys_initcall(pps_init); diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h index 78c8ac4951b5..c7abce28ed29 100644 --- a/include/linux/pps_kernel.h +++ b/include/linux/pps_kernel.h @@ -56,8 +56,7 @@ struct pps_device { =20 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; }; --=20 2.45.2