From: Jason Gunthorpe <jgg@ziepe.ca>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Greg KH <greg@kroah.com>,
Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
James Bottomley <James.Bottomley@hansenpartnership.com>,
Mark Brown <broonie@kernel.org>,
Linus Walleij <linus.walleij@linaro.org>,
Roland Dreier <roland@kernel.org>,
Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>,
ksummit@lists.linux.dev, Daniel Vetter <daniel@ffwll.ch>
Subject: Re: cdev/devm_* issues (was Re: [TECH TOPIC] Rust for Linux)
Date: Mon, 12 Jul 2021 10:42:33 -0300 [thread overview]
Message-ID: <20210712134233.GA141137@ziepe.ca> (raw)
In-Reply-To: <20210710070910.GA2190@kadam>
On Sat, Jul 10, 2021 at 10:09:10AM +0300, Dan Carpenter wrote:
> The other thing which is quite tricky to get right is kobj releases. I
> have a bunch of these static checker warning but they're annoying
> because they're not something which really affects real life so I can't
> be bothered to fix or report them.
I'm not a security researcher, but I often wonder if there are ways
root could exploit these kinds of error unwind bugs during module
loading to exploit the kernel? That is a relevant concern in the
higher integrity modes.
> 1045 if (nr != -1 && nr != vdev->num && warn_if_nr_in_use)
> 1046 pr_warn("%s: requested %s%d, got %s\n", __func__,
> 1047 name_base, nr, video_device_node_name(vdev));
> 1048
> 1049 /* Increase v4l2_device refcount */
> 1050 v4l2_device_get(vdev->v4l2_dev);
> 1051
> 1052 /* Part 5: Register the entity. */
> 1053 ret = video_register_media_controller(vdev);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> But then there is no way to clean up from this is
> video_register_media_controller() fails. If we call put_device() it
> would lead to use after frees in the callers. We just have to ignore
> the error.
Eg if we ignore these errors and keep going is the kernel object in a
bad state that userspace can exploit?
The solution here is the same as the other cases, use device_add() not
device_register(), put a device_initialize() immediately after the
allocation of vdev and audit/fix the release so it can work on
partially initialized objects.
The idea of a video_register_device() is just not right in a subsystem
so complicated the 'init & register' pattern is a shortcut to save a
few lines in simple drivers. It should never be baked into a subsystem
like this and it certainly shouldn't work the opposide of how
device_register() works: "if video_register_device fails, the
release() callback of &struct video_device structure is *not* called"
Which is why the release function assignment is out of order, and it
is forced to print a warning in a certain error flow :\
> It's a minor thing, but it's so frustrating.
Yes :(
And as others mentioned devm has its own set of bug classes - I've
seen issues with the sequencing of the devm unloads.. Especially when
work queues get involved.
Jason
next prev parent reply other threads:[~2021-07-12 13:42 UTC|newest]
Thread overview: 200+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-25 22:09 [TECH TOPIC] Rust for Linux Miguel Ojeda
2021-07-05 23:51 ` Linus Walleij
2021-07-06 4:30 ` Leon Romanovsky
2021-07-06 9:55 ` Linus Walleij
2021-07-06 10:16 ` Geert Uytterhoeven
2021-07-06 17:59 ` Linus Walleij
2021-07-06 18:36 ` Miguel Ojeda
2021-07-06 19:12 ` Linus Walleij
2021-07-06 21:32 ` Miguel Ojeda
2021-07-07 14:10 ` Arnd Bergmann
2021-07-07 15:28 ` Miguel Ojeda
2021-07-07 15:50 ` Andrew Lunn
2021-07-07 16:34 ` Miguel Ojeda
2021-07-07 16:55 ` Arnd Bergmann
2021-07-07 17:54 ` Miguel Ojeda
2021-07-06 10:22 ` Leon Romanovsky
2021-07-06 14:30 ` Miguel Ojeda
2021-07-06 14:32 ` Miguel Ojeda
2021-07-06 15:03 ` Sasha Levin
2021-07-06 15:33 ` Miguel Ojeda
2021-07-06 15:42 ` Laurent Pinchart
2021-07-06 16:09 ` Mike Rapoport
2021-07-06 18:29 ` Miguel Ojeda
2021-07-06 18:38 ` Laurent Pinchart
2021-07-06 19:45 ` Steven Rostedt
2021-07-06 19:59 ` Miguel Ojeda
2021-07-06 18:53 ` Sasha Levin
2021-07-06 21:50 ` Miguel Ojeda
2021-07-07 4:57 ` Leon Romanovsky
2021-07-07 13:39 ` Alexandre Belloni
2021-07-07 13:50 ` Miguel Ojeda
2021-07-06 18:26 ` Linus Walleij
2021-07-06 19:11 ` Miguel Ojeda
2021-07-06 19:13 ` Johannes Berg
2021-07-06 19:43 ` Miguel Ojeda
2021-07-06 10:20 ` James Bottomley
2021-07-06 14:55 ` Miguel Ojeda
2021-07-06 15:01 ` Sasha Levin
2021-07-06 15:36 ` Miguel Ojeda
2021-07-09 10:02 ` Marco Elver
2021-07-09 16:02 ` Miguel Ojeda
2021-07-06 18:09 ` Linus Walleij
2021-07-06 14:24 ` Miguel Ojeda
2021-07-06 14:33 ` Laurent Pinchart
2021-07-06 14:56 ` Leon Romanovsky
2021-07-06 15:29 ` Miguel Ojeda
2021-07-07 4:38 ` Leon Romanovsky
2021-07-06 20:00 ` Roland Dreier
2021-07-06 20:36 ` Linus Walleij
2021-07-06 22:00 ` Laurent Pinchart
2021-07-07 7:27 ` Julia Lawall
2021-07-07 7:45 ` Greg KH
2021-07-07 7:52 ` James Bottomley
2021-07-07 13:49 ` Miguel Ojeda
2021-07-07 14:08 ` James Bottomley
2021-07-07 15:15 ` Miguel Ojeda
2021-07-07 15:44 ` Greg KH
2021-07-07 17:01 ` Wedson Almeida Filho
2021-07-07 17:20 ` Greg KH
2021-07-07 19:19 ` Wedson Almeida Filho
2021-07-07 20:38 ` Jan Kara
2021-07-07 23:09 ` Wedson Almeida Filho
2021-07-08 6:11 ` Greg KH
2021-07-08 13:36 ` Wedson Almeida Filho
2021-07-08 18:51 ` Greg KH
2021-07-08 19:31 ` Andy Lutomirski
2021-07-08 19:35 ` Geert Uytterhoeven
2021-07-08 21:56 ` Andy Lutomirski
2021-07-08 19:49 ` Linus Walleij
2021-07-08 20:34 ` Miguel Ojeda
2021-07-08 22:13 ` Linus Walleij
2021-07-09 7:24 ` Geert Uytterhoeven
2021-07-19 12:24 ` Wedson Almeida Filho
2021-07-19 13:15 ` Wedson Almeida Filho
2021-07-19 14:02 ` Arnd Bergmann
2021-07-19 14:13 ` Linus Walleij
2021-07-19 21:32 ` Arnd Bergmann
2021-07-19 21:33 ` Arnd Bergmann
2021-07-20 1:46 ` Miguel Ojeda
2021-07-20 6:43 ` Johannes Berg
2021-07-19 14:43 ` Geert Uytterhoeven
2021-07-19 18:24 ` Miguel Ojeda
2021-07-19 18:47 ` Steven Rostedt
2021-07-19 14:54 ` Miguel Ojeda
2021-07-19 17:32 ` Wedson Almeida Filho
2021-07-19 21:31 ` Arnd Bergmann
2021-07-19 17:37 ` Miguel Ojeda
2021-07-19 16:02 ` Vegard Nossum
2021-07-19 17:45 ` Miguel Ojeda
2021-07-19 17:54 ` Miguel Ojeda
2021-07-19 18:06 ` Wedson Almeida Filho
2021-07-19 19:37 ` Laurent Pinchart
2021-07-19 21:09 ` Wedson Almeida Filho
2021-07-20 23:54 ` Laurent Pinchart
2021-07-21 1:33 ` Andy Lutomirski
2021-07-21 1:42 ` Laurent Pinchart
2021-07-21 13:54 ` Linus Walleij
2021-07-21 14:13 ` Wedson Almeida Filho
2021-07-21 14:19 ` Linus Walleij
2021-07-22 11:33 ` Wedson Almeida Filho
2021-07-23 0:45 ` Linus Walleij
2021-07-21 4:39 ` Wedson Almeida Filho
2021-07-23 1:04 ` Laurent Pinchart
2021-07-21 4:23 ` Wedson Almeida Filho
2021-07-23 1:13 ` Laurent Pinchart
2021-07-19 22:57 ` Alexandre Belloni
2021-07-20 7:15 ` Miguel Ojeda
2021-07-20 9:39 ` Alexandre Belloni
2021-07-20 12:10 ` Miguel Ojeda
2021-07-19 13:53 ` Linus Walleij
2021-07-19 14:42 ` Wedson Almeida Filho
2021-07-19 22:16 ` Linus Walleij
2021-07-20 1:20 ` Wedson Almeida Filho
2021-07-20 13:21 ` Andrew Lunn
2021-07-20 13:38 ` Miguel Ojeda
2021-07-20 14:04 ` Andrew Lunn
2021-07-20 13:55 ` Greg KH
2021-07-20 1:21 ` Miguel Ojeda
2021-07-20 16:00 ` Mark Brown
2021-07-20 22:42 ` Linus Walleij
2021-07-19 14:43 ` Miguel Ojeda
2021-07-19 15:15 ` Andrew Lunn
2021-07-19 15:43 ` Miguel Ojeda
2021-07-09 7:03 ` Viresh Kumar
2021-07-09 17:06 ` Mark Brown
2021-07-09 17:43 ` Miguel Ojeda
2021-07-10 9:53 ` Jonathan Cameron
2021-07-10 20:09 ` Kees Cook
2021-07-08 13:55 ` Miguel Ojeda
2021-07-08 14:58 ` Greg KH
2021-07-08 15:02 ` Mark Brown
2021-07-08 16:38 ` Andy Lutomirski
2021-07-08 18:01 ` Greg KH
2021-07-08 18:00 ` Miguel Ojeda
2021-07-08 18:44 ` Greg KH
2021-07-08 23:09 ` Miguel Ojeda
2021-07-08 7:20 ` Geert Uytterhoeven
2021-07-08 13:41 ` Wedson Almeida Filho
2021-07-08 13:43 ` Geert Uytterhoeven
2021-07-08 13:54 ` Wedson Almeida Filho
2021-07-08 14:16 ` Geert Uytterhoeven
2021-07-08 14:24 ` Wedson Almeida Filho
2021-07-09 7:04 ` Jerome Glisse
2021-07-08 14:04 ` Miguel Ojeda
2021-07-08 14:18 ` Geert Uytterhoeven
2021-07-08 14:28 ` Miguel Ojeda
2021-07-08 14:33 ` Geert Uytterhoeven
2021-07-08 14:35 ` Miguel Ojeda
2021-07-09 11:55 ` Geert Uytterhoeven
2021-07-08 16:07 ` Andy Lutomirski
2021-07-07 20:58 ` Miguel Ojeda
2021-07-07 21:47 ` Laurent Pinchart
2021-07-07 22:44 ` Miguel Ojeda
2021-07-07 17:01 ` Miguel Ojeda
2021-07-07 10:50 ` Mark Brown
2021-07-07 10:56 ` Julia Lawall
2021-07-07 11:27 ` James Bottomley
2021-07-07 11:34 ` James Bottomley
2021-07-07 12:20 ` Greg KH
2021-07-07 12:38 ` James Bottomley
2021-07-07 12:45 ` Greg KH
2021-07-07 17:17 ` Laurent Pinchart
2021-07-08 6:49 ` cdev/devm_* issues (was Re: [TECH TOPIC] Rust for Linux) Greg KH
2021-07-08 8:23 ` Laurent Pinchart
2021-07-08 23:06 ` Linus Walleij
2021-07-09 0:02 ` Dan Williams
2021-07-09 16:53 ` Wedson Almeida Filho
2021-07-13 8:59 ` Linus Walleij
[not found] ` <CAHp75VfW7PxAyU=eYPNWFU_oUY=aStz-4W5gX87KSo402YhMXQ@mail.gmail.com>
2021-07-21 13:46 ` Linus Walleij
2021-07-21 15:49 ` Andy Shevchenko
2021-07-10 7:09 ` Dan Carpenter
2021-07-12 13:42 ` Jason Gunthorpe [this message]
2021-07-15 9:54 ` Daniel Vetter
2021-07-21 9:08 ` Dan Carpenter
2021-07-22 9:56 ` Daniel Vetter
2021-07-22 10:09 ` Dan Carpenter
2021-07-08 9:08 ` [TECH TOPIC] Rust for Linux Mauro Carvalho Chehab
2021-07-10 16:42 ` Laurent Pinchart
2021-07-10 17:18 ` Andy Lutomirski
2021-07-07 15:17 ` Mark Brown
2021-07-06 21:45 ` Bart Van Assche
2021-07-06 23:08 ` Stephen Hemminger
2021-07-07 2:41 ` Bart Van Assche
2021-07-07 18:57 ` Linus Torvalds
2021-07-07 20:32 ` Bart Van Assche
2021-07-07 20:39 ` Linus Torvalds
2021-07-07 21:40 ` Laurent Pinchart
2021-07-08 7:22 ` Geert Uytterhoeven
2021-07-07 21:02 ` Laurent Pinchart
2021-07-07 22:11 ` Miguel Ojeda
2021-07-07 22:43 ` Laurent Pinchart
2021-07-07 23:21 ` Miguel Ojeda
2021-07-07 23:40 ` Laurent Pinchart
2021-07-08 0:27 ` Miguel Ojeda
2021-07-08 0:56 ` Laurent Pinchart
2021-07-08 6:26 ` Alexey Dobriyan
2021-07-06 19:05 ` Bart Van Assche
2021-07-06 19:27 ` Miguel Ojeda
2021-07-07 15:48 ` Steven Rostedt
2021-07-07 16:44 ` Miguel Ojeda
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=20210712134233.GA141137@ziepe.ca \
--to=jgg@ziepe.ca \
--cc=James.Bottomley@hansenpartnership.com \
--cc=broonie@kernel.org \
--cc=dan.carpenter@oracle.com \
--cc=daniel@ffwll.ch \
--cc=greg@kroah.com \
--cc=ksummit@lists.linux.dev \
--cc=laurent.pinchart@ideasonboard.com \
--cc=linus.walleij@linaro.org \
--cc=miguel.ojeda.sandonis@gmail.com \
--cc=roland@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).