All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chad Aldreda <ca@modelmiser.com>
To: "Pali Rohár" <pali@kernel.org>
Cc: "Hans de Goede" <hansg@kernel.org>,
	"Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>,
	platform-driver-x86@vger.kernel.org, stable@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] platform/x86: dell-smo8800: keep the state until the last file is closed
Date: Tue, 10 Nov 2026 22:27:59 +0000	[thread overview]
Message-ID: <20261110222752.2902827-2-ca@modelmiser.com> (raw)
In-Reply-To: <20261110210359.upmeyc4b4xksfva2@pali>

On Tue, 10 Nov 2026, Pali Rohár wrote:
> Hello, I have not checked this patch too deeply, but I have feeling that
> something is probably wrong if we need to add a new manual refcounter
> around a whole device.

Fair question. The count covers only the lifetime of struct
smo8800_device: one reference while the driver is bound and one per
open /dev/freefall file. It is needed because the struct comes from
devm_kzalloc(), which frees it when the driver unbinds, while a file
opened before the unbind stays open and still reaches it through
file->private_data.

> Anyway, this freefall logic was originally copied from the lis3lv02d.c
> driver, which now lives in the file:
> linux/drivers/misc/lis3lv02d/lis3lv02d.c
> So if in the freefall device of dell-smo8800.c is some error then
> probably the same issue would be in the freefall device of lis3lv02d.c.

Thanks for the pointer. lis3lv02d does not have this use-after-free: its
freefall state is the static global lis3_dev, which is never freed, and
its file operations reach it through the miscdevice embedded there. It
does share the other half: a read() blocked at remove is not woken there
either, but with static state that only leaves the reader asleep until a
signal, without touching freed memory.

Making smo8800_device static like lis3_dev would also fix the
use-after-free. I kept it per device because with static state a file
left open across an unbind and bind keeps the device marked open, so new
opens get -EBUSY until it is closed, and the old file sees the new
binding's events, as in lis3lv02d. Either way I would keep the wakeup
at remove, so that a blocked read() returns -ENODEV instead of sleeping
until a signal. If you prefer the static version, I can send a v2 doing
that.

Chad

This reply was drafted with an AI assistant (Claude Opus 5.5).


      reply	other threads:[~2026-11-10 22:28 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-07 20:49 [PATCH] platform/x86: dell-smo8800: keep the state until the last file is closed Chad Aldreda
2026-11-09  8:19 ` Chad Aldreda
2026-11-10 21:03 ` Pali Rohár
2026-11-10 22:27   ` Chad Aldreda [this message]

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=20261110222752.2902827-2-ca@modelmiser.com \
    --to=ca@modelmiser.com \
    --cc=hansg@kernel.org \
    --cc=ilpo.jarvinen@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pali@kernel.org \
    --cc=platform-driver-x86@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /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.