* Re: [RFC PATCH v2] media: Virtual camera driver
From: Laurent Pinchart @ 2026-02-03 20:57 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Sakari Ailus, linux-media, jani.nikula, anisse, oleksandr,
linux-integrity, Mauro Carvalho Chehab, Hans Verkuil,
Jacopo Mondi, Ricardo Ribalda, open list
In-Reply-To: <aYFRO1XdtEUkbSCg@kernel.org>
Hello Jarkko,
On Tue, Feb 03, 2026 at 03:36:59AM +0200, Jarkko Sakkinen wrote:
> On Tue, Feb 03, 2026 at 02:10:15AM +0200, Jarkko Sakkinen wrote:
> > On Tue, Feb 03, 2026 at 12:50:06AM +0200, Sakari Ailus wrote:
> > > On Mon, Feb 02, 2026 at 10:44:21PM +0200, Jarkko Sakkinen wrote:
> > > > Already a quick Google survey backs strongly that OOT drivers (e.g.,
> > > > v4l2loopback) are the defacto solution for streaming phone cameras in
> > > > video conference calls, which puts confidential discussions at risk.
> > >
> > > As I think it was pointed out in review comments for v1, the reason behind
> > > using v4l2loopback is the use of a downstream driver, which itself is a
> > > source of a security risk. If I understand correctly, supporting this
> > > (proprietary/downstream vendor drivers) would be the main use case this
> > > driver serves? Should this downstream driver be upstreamed to alleviate the
> > > security risks, the need for v4l2loopback or similar drivers presumably
> > > disappears.
> >
> > My goal is not to proactively support proprietary drivers, and I don't
> > know how to measure such incentive or risk, when it comes to video
> > drivers.
> >
> > And besides there is e.g. FUSE.
> >
> > > Another of the downsides of such proprietary/downstream solutions is they
> > > can never be properly integrated into the Linux ecosystem so functionality
> > > will remain spotty (limited to specific systems and specific releases of
> > > specific distributions) at best.
> > >
> > > In other words, this driver appears to be orthogonal to solving either of
> > > the above two problems the proprietary/downstream solutions have.
> > >
> > > From the Open Source libcamera based camera software stack point of view
> > > there doesn't seem to be a need for v4l2loopback or another similar driver.
> > > The two main reasons for this is that (1) there's no need for glueing
> > > something separate together like this and (2) V4L2 isn't a great
> > > application interface for cameras -- use libcamera or Pipewire instead.
> >
> > While I get this argument isolated, it does not match the observed
> > reality, and does not provide tools to address the core issue. I
> > will be in my grave before I've fixed the world like you are
> > suggesting :-)
I really hope we'll provide a solution much faster than that :-)
> > Like, first off, where would I use libcamera or Pipewire? There's
> > no well-defined target other than kernel in this problem.
PipeWire is becoming the de facto media server on desktop systems, for
both audio and video. It has been shipped by distributions for a while
for audio, and is the core component that allows screen capture (and
therefore screen sharing in video conferencing) on Wayland-based
systems. For video, PipeWire support has most notably been integrated in
WebRTC, used by both Firefox and Chrome. The number of applications
using PipeWire is growing, OBS has recently received support for
PipeWire sources for instance. If you need to use it in an application
that requires a V4L2 capture device, the pw-v4l2 script emulates the
V4L2 API to provide a quick stopgap measure until applications get
native PipeWire support.
libcamera solves an orthogonal problem, which is control of raw camera
sensors and ISPs typically found in mobile and embedded devices, and now
increasingly in laptops as well (Intel IPU3, IPU4, IPU6 and IPU7).
Applications typically don't use libcamera directly, but interface it
with GStreamer (libcamerasrc element) or PipeWire (which has native
libcamera support).
While I understand that libcamera and PipeWire may be quite new for a
large number of users, the ecosystem is moving in that direction, and
both projects are very active.
> > > > It can be also claimed that there's enough OOT usage in the wild that
> > > > possible security bugs could be considered as potential zerodays for the
> > > > benefit of malicious actors.
> > > >
> > > > The situation has been stagnated for however many years, which is
> > > > unsastainable situation, and it further factors potential security
> > > > risks. Therefore, a driver is needed to address the popular use case.
> > > >
> > > > vcam is a DMA-BUF backed virtual camera driver capable of creating video
> > > > capture devices to which data can be streamed through /dev/vcam after
> > > > calling VCAM_IOC_CREATE. Frames are pushed with VCAM_IOC_QUEUE and recycled
> > > > with VCAM_IOC_DEQUEUE. Zero-copy semantics are supported for shared DMA-BUF
> > > > between capture and output.
> > > >
> > > > This enables efficient implementation of software, which can manage network
> > > > video streams from phone cameras, and map those streams to video devices.
> > >
> > > I'd really try to avoid involving V4L2 in-kernel implementation when the
> > > source of the video is network. V4L2 is meant to be used (when it comes to
> > > video) for interfacing video related hardware such as cameras, ISPs and
> > > codecs. There are limited number of video output related devices, too, but
> > > network is something quite different from these.
> >
> > I'd look at the usage patterns in the field too. It is pretty obvious
> > that there is a significant gap what users want and expect when it
> > comes to this debate.
>
> As for the patch itself, it is RFC i.e., not request for immediate
> merge. I sent v2 quickly primarily to address the motivation part
> properly. I'll phase this down a bit, and rework on issues in the uAPI I
> observed (and remarked in a response to this patch) etc., and generally
> give people some time to digest.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC PATCH v2] media: Virtual camera driver
From: Laurent Pinchart @ 2026-02-03 21:09 UTC (permalink / raw)
To: Jani Nikula
Cc: johannes.goede, Jarkko Sakkinen, linux-media, anisse, oleksandr,
linux-integrity, Mauro Carvalho Chehab, Hans Verkuil,
Sakari Ailus, Jacopo Mondi, Ricardo Ribalda, open list
In-Reply-To: <906e9bd379456d4a861dc58d1504ee8546d8997d@intel.com>
Hi Jarkko,
On Tue, Feb 03, 2026 at 03:16:24PM +0200, Jani Nikula wrote:
> On Tue, 03 Feb 2026, johannes.goede@oss.qualcomm.com wrote:
> > On 2-Feb-26 21:44, Jarkko Sakkinen wrote:
> >> Already a quick Google survey backs strongly that OOT drivers (e.g.,
> >> v4l2loopback) are the defacto solution for streaming phone cameras in
> >> video conference calls, which puts confidential discussions at risk.
> >>
> >> It can be also claimed that there's enough OOT usage in the wild that
> >> possible security bugs could be considered as potential zerodays for the
> >> benefit of malicious actors.
> >>
> >> The situation has been stagnated for however many years, which is
> >> unsastainable situation, and it further factors potential security
> >> risks. Therefore, a driver is needed to address the popular use case.
> >>
> >> vcam is a DMA-BUF backed virtual camera driver capable of creating video
> >> capture devices to which data can be streamed through /dev/vcam after
> >> calling VCAM_IOC_CREATE. Frames are pushed with VCAM_IOC_QUEUE and recycled
> >> with VCAM_IOC_DEQUEUE. Zero-copy semantics are supported for shared DMA-BUF
> >> between capture and output.
> >>
> >> This enables efficient implementation of software, which can manage network
> >> video streams from phone cameras, and map those streams to video devices.
> >>
> >> PipeWire or any other specific pick of userspace software cannot really
> >> address the issue at scale, as e.g., the use of v4l2loopback is both wide
> >> and scattered.
> >
> > I appreciate your efforts here and if I understand things correctly
> > your main goal is to allow people to use the camera of there mobile
> > phone for e.g. video-conferencing on a standard linux distro
> > (Debian/Fedora/Arch) laptop/desktop right ?
> >
> > The problem is that what you're suggesting is basically a much
> > improved (using dma-buf is way better) v4l2-loopback driver and
> > v4l2-loopback has been blocked from getting merged into the kernel
> > because besides the mobile-phone camera use, the other main use-case
> > is to allow running proprietary camera stacks like Intel's proprietary
> > camerastack and then presenting that to userspace as a standard v4l2
> > cam so that userspace apps will just work.
> >
> > Personally I think that there are enough valid use-cases for something
> > like your proposed vcam driver, also for e.g. CI purposes that I think
> > that maybe the V4L2 maintainers should reconsider. But this is not my
> > call to make and with both Laurent (in the v1 thread) and Sakari pretty
> > much having NACK-ed this, I do not think this is going to go very far
> > if you want a mainline solution.
> >
> > As Sakari mentioned in this day and age raw V4L2 access is really not
> > the best API to acccess cameras. Especially modern smartphone like
> > CSI2 MIPI cameras which consist of quite a complex graph of building
> > blocks which need to be configured to work together to get a picture.
> >
> > So now even when running directly on the hardware (vs the network case)
> > apps can no longer directly access v4l2 devices because of the complexity.
> >
> > This is already the case on newer x86_64 laptops with MIPI cameras
> > instead of USB UVC cams and also on phones running postmarketos.
> >
> > The community concensus is that the solution here is for apps to
> > access cameras through pipewire. Together with the shift of laptops
> > cameras from UVC to "raw" MIPI cameras there also is a shift to
> > running applications sandboxed as flatpacks because of the changing
> > "cyber" security landscape. This is why pipewire was chosen because
> > it also solves the accessing cameras from a sandbox issue.
> >
> > Both Firefox and chrome(ium) already support pipewire for cameras
> > OOTB. In chrome it requires setting a config setting and maybe for
> > upstream Firefox too (it is enabled in Fedora's Firefox by default).
> >
> > Given that pipewire is seen as the way to access all cameras in
> > the future and that we are already moving in that direction (e.g.
> > Fedora has been shipping this OOTB since Fedora 42) IMHO your time
> > would be better spend helping out there.
> >
> > E.g. add support to popular software for sharing smartphone cameras
> > to represent the frames received over the network as a video sources
> > in pipewire and check if this works in Firefox + Chrome and write
> > patches or file issues for other apps to also add pipewire camera
> > access support.
> >
> > As was mentioned in the earlier thread for making the camera show
> > up as a source in pipewire you could try using the pipewire sink
> > element in gstreamer, assuming you can make the phone-cam available
> > as a gstreamer src element, you may be able to prototype things
> > then using just a gst-launch cmd from the shell.
>
> I described my use case at [1]. Basically have OBS Studio output to a
> virtual camera that can be used by other apps. A very common use case
> for v4l2-loopback. Nothing to do with phone cameras or proprietary
> drivers.
>
> So there appears to be an OBS Studio PR [2] to enable pipewire virtual
> camera, open for about five years now.
Thanks for the pointer, I wasn't aware of that.
> And it still won't work with any
> legacy apps that need the traditional camera device, until they're
> migrated to pipewire too, if ever.
Legacy apps that only support /dev/video* will increasingly not work
with mobile, embedded and even laptop devices as the industry is using
raw sensors with ISPs everywhere, requiring libcamera in userspace. In
the meantime, both libcamera and PipeWire provide LD_PRELOAD-able
libraries that emulate V4L2 entirely in userspace on top of the
respective project, for those legacy applications (wrapped in scripts
called libcamerify and pw-v4l2).
> Maybe it would take non-trivial effort to port OBS Studio to vcam too, I
> haven't looked, and I'm no expert anyway.
It would require some work, as the vcam driver uses a custom API. I
believe that the effort would be better spent on completing PipeWire
support.
> 'apt install v4l2loopback-dkms' remains the primary option for most
> people for the foreseeable future.
>
> [1] https://lore.kernel.org/r/c6b719fdecbfc8a1c15a197b6ae51da56ed54f63@intel.com
>
> [2] https://github.com/obsproject/obs-studio/pull/5377
>
> >> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> >> ---
> >> v2:
> >> - Added motivation based on feedback to v1.
> >> - Provide a list of allowed modes for /dev/videoX in VCAM_IOC_CREATE.
> >> - Merged VCAM_IOC_WAIT and VCAM_IOC_STATUS.
> >> - Return state with operation flags in VCAM_IOC_WAIT.
> >> - Test program: https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/vcam-test.git
> >> ---
> >> .../driver-api/media/drivers/index.rst | 1 +
> >> .../driver-api/media/drivers/vcam.rst | 16 +
> >> MAINTAINERS | 8 +
> >> drivers/media/Kconfig | 13 +
> >> drivers/media/Makefile | 1 +
> >> drivers/media/vcam.c | 1935 +++++++++++++++++
> >> include/uapi/linux/vcam.h | 141 ++
> >> 7 files changed, 2115 insertions(+)
> >> create mode 100644 Documentation/driver-api/media/drivers/vcam.rst
> >> create mode 100644 drivers/media/vcam.c
> >> create mode 100644 include/uapi/linux/vcam.h
[snip]
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC PATCH v2] media: Virtual camera driver
From: Jarkko Sakkinen @ 2026-02-03 21:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Sakari Ailus, linux-media, jani.nikula, anisse, oleksandr,
linux-integrity, Mauro Carvalho Chehab, Hans Verkuil,
Jacopo Mondi, Ricardo Ribalda, open list
In-Reply-To: <20260203205742.GB11369@killaraus>
On Tue, Feb 03, 2026 at 10:57:42PM +0200, Laurent Pinchart wrote:
> Hello Jarkko,
>
> On Tue, Feb 03, 2026 at 03:36:59AM +0200, Jarkko Sakkinen wrote:
> > On Tue, Feb 03, 2026 at 02:10:15AM +0200, Jarkko Sakkinen wrote:
> > > On Tue, Feb 03, 2026 at 12:50:06AM +0200, Sakari Ailus wrote:
> > > > On Mon, Feb 02, 2026 at 10:44:21PM +0200, Jarkko Sakkinen wrote:
> > > > > Already a quick Google survey backs strongly that OOT drivers (e.g.,
> > > > > v4l2loopback) are the defacto solution for streaming phone cameras in
> > > > > video conference calls, which puts confidential discussions at risk.
> > > >
> > > > As I think it was pointed out in review comments for v1, the reason behind
> > > > using v4l2loopback is the use of a downstream driver, which itself is a
> > > > source of a security risk. If I understand correctly, supporting this
> > > > (proprietary/downstream vendor drivers) would be the main use case this
> > > > driver serves? Should this downstream driver be upstreamed to alleviate the
> > > > security risks, the need for v4l2loopback or similar drivers presumably
> > > > disappears.
> > >
> > > My goal is not to proactively support proprietary drivers, and I don't
> > > know how to measure such incentive or risk, when it comes to video
> > > drivers.
> > >
> > > And besides there is e.g. FUSE.
> > >
> > > > Another of the downsides of such proprietary/downstream solutions is they
> > > > can never be properly integrated into the Linux ecosystem so functionality
> > > > will remain spotty (limited to specific systems and specific releases of
> > > > specific distributions) at best.
> > > >
> > > > In other words, this driver appears to be orthogonal to solving either of
> > > > the above two problems the proprietary/downstream solutions have.
> > > >
> > > > From the Open Source libcamera based camera software stack point of view
> > > > there doesn't seem to be a need for v4l2loopback or another similar driver.
> > > > The two main reasons for this is that (1) there's no need for glueing
> > > > something separate together like this and (2) V4L2 isn't a great
> > > > application interface for cameras -- use libcamera or Pipewire instead.
> > >
> > > While I get this argument isolated, it does not match the observed
> > > reality, and does not provide tools to address the core issue. I
> > > will be in my grave before I've fixed the world like you are
> > > suggesting :-)
>
> I really hope we'll provide a solution much faster than that :-)
>
> > > Like, first off, where would I use libcamera or Pipewire? There's
> > > no well-defined target other than kernel in this problem.
>
> PipeWire is becoming the de facto media server on desktop systems, for
> both audio and video. It has been shipped by distributions for a while
> for audio, and is the core component that allows screen capture (and
> therefore screen sharing in video conferencing) on Wayland-based
> systems. For video, PipeWire support has most notably been integrated in
> WebRTC, used by both Firefox and Chrome. The number of applications
> using PipeWire is growing, OBS has recently received support for
> PipeWire sources for instance. If you need to use it in an application
> that requires a V4L2 capture device, the pw-v4l2 script emulates the
> V4L2 API to provide a quick stopgap measure until applications get
> native PipeWire support.
>
> libcamera solves an orthogonal problem, which is control of raw camera
> sensors and ISPs typically found in mobile and embedded devices, and now
> increasingly in laptops as well (Intel IPU3, IPU4, IPU6 and IPU7).
> Applications typically don't use libcamera directly, but interface it
> with GStreamer (libcamerasrc element) or PipeWire (which has native
> libcamera support).
>
> While I understand that libcamera and PipeWire may be quite new for a
> large number of users, the ecosystem is moving in that direction, and
> both projects are very active.
Thanks for the information and I take this into account when/if considering
any updates. The response is so informative that I need to purge this a
bit (thank you for that) :-) This does not disregard your response but
personally I'm not have huge a fan of LD_PRELOAD style compatibility
wrappers.
BR, Jarkko
^ permalink raw reply
* Re: [RFC PATCH v2] media: Virtual camera driver
From: Laurent Pinchart @ 2026-02-03 21:18 UTC (permalink / raw)
To: johannes.goede
Cc: Jani Nikula, Jarkko Sakkinen, linux-media, anisse, oleksandr,
linux-integrity, Mauro Carvalho Chehab, Hans Verkuil,
Sakari Ailus, Jacopo Mondi, Ricardo Ribalda, open list
In-Reply-To: <c6b5832f-d7b3-4bc2-834e-4ce7b0c0b4cd@oss.qualcomm.com>
On Tue, Feb 03, 2026 at 03:19:13PM +0100, johannes.goede@oss.qualcomm.com wrote:
> On 3-Feb-26 14:20, Jani Nikula wrote:
> > On Tue, 03 Feb 2026, johannes.goede@oss.qualcomm.com wrote:
> >> The problem is that what you're suggesting is basically a much
> >> improved (using dma-buf is way better) v4l2-loopback driver and
> >> v4l2-loopback has been blocked from getting merged into the kernel
> >> because besides the mobile-phone camera use, the other main use-case
> >> is to allow running proprietary camera stacks like Intel's proprietary
> >> camerastack and then presenting that to userspace as a standard v4l2
> >> cam so that userspace apps will just work.
> >
> > ...
> >
> >> The community concensus is that the solution here is for apps to
> >> access cameras through pipewire. Together with the shift of laptops
> >> cameras from UVC to "raw" MIPI cameras there also is a shift to
> >> running applications sandboxed as flatpacks because of the changing
> >> "cyber" security landscape. This is why pipewire was chosen because
> >> it also solves the accessing cameras from a sandbox issue.
> >
> > Why is v4l2-loopback problematic from the perspective of facilitating
> > running proprietary camera stacks, but pipewire isn't?
>
> Once pipewire mostly works everywhere for camera access then indeed
> this will allow proprietary stacks to present themselves as a pipewire src.
> For now though most proprietary stacks seem to prefer v4l2loopback because
> pipewire is not supported as camera source yet by a lot of apps.
I agree, that's a perfectly valid assessment of the situation.
> As I indicated in my original email personally I'm a bit divided on
> whether a virtual camera driver should be kept out of the kernel
> to not promote proprietary userspace stacks, but this is not my call.
Even disregarding that argument, the camera ecosystem is moving towards
implementing those use cases entirely in userspace with PipeWire. I
believe that merging a new kernel driver for this purpose, especially
one that exposes a custom API on its sink side and would therefore
require developing support in all source applications, goes against the
direction we're taking overall.
> OTHO evdi: https://github.com/DisplayLink/evdi has been kept out
> of the kernel for pretty much the same reasons by the drm/kms folks.
>
> At least AFAIK there still is no way to present virtual kms capable
> display outputs backed by userspace in the kernel.
>
> I completely understand where you're coming from wrt v4l2-loopback
> support (or something equivalent) but asking for this really is
> the same as asking for the evdi driver to get merged, which AFAIK
> has been blocked for the reason of avoiding proprietary userspace
> display output drivers (I guess there might be technical reasons too).
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC PATCH v2] media: Virtual camera driver
From: Laurent Pinchart @ 2026-02-03 21:21 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Sakari Ailus, linux-media, jani.nikula, anisse, oleksandr,
linux-integrity, Mauro Carvalho Chehab, Hans Verkuil,
Jacopo Mondi, Ricardo Ribalda, open list
In-Reply-To: <aYJkZEV6mX9Kl56X@kernel.org>
On Tue, Feb 03, 2026 at 11:11:19PM +0200, Jarkko Sakkinen wrote:
> On Tue, Feb 03, 2026 at 10:57:42PM +0200, Laurent Pinchart wrote:
> > On Tue, Feb 03, 2026 at 03:36:59AM +0200, Jarkko Sakkinen wrote:
> > > On Tue, Feb 03, 2026 at 02:10:15AM +0200, Jarkko Sakkinen wrote:
> > > > On Tue, Feb 03, 2026 at 12:50:06AM +0200, Sakari Ailus wrote:
> > > > > On Mon, Feb 02, 2026 at 10:44:21PM +0200, Jarkko Sakkinen wrote:
> > > > > > Already a quick Google survey backs strongly that OOT drivers (e.g.,
> > > > > > v4l2loopback) are the defacto solution for streaming phone cameras in
> > > > > > video conference calls, which puts confidential discussions at risk.
> > > > >
> > > > > As I think it was pointed out in review comments for v1, the reason behind
> > > > > using v4l2loopback is the use of a downstream driver, which itself is a
> > > > > source of a security risk. If I understand correctly, supporting this
> > > > > (proprietary/downstream vendor drivers) would be the main use case this
> > > > > driver serves? Should this downstream driver be upstreamed to alleviate the
> > > > > security risks, the need for v4l2loopback or similar drivers presumably
> > > > > disappears.
> > > >
> > > > My goal is not to proactively support proprietary drivers, and I don't
> > > > know how to measure such incentive or risk, when it comes to video
> > > > drivers.
> > > >
> > > > And besides there is e.g. FUSE.
> > > >
> > > > > Another of the downsides of such proprietary/downstream solutions is they
> > > > > can never be properly integrated into the Linux ecosystem so functionality
> > > > > will remain spotty (limited to specific systems and specific releases of
> > > > > specific distributions) at best.
> > > > >
> > > > > In other words, this driver appears to be orthogonal to solving either of
> > > > > the above two problems the proprietary/downstream solutions have.
> > > > >
> > > > > From the Open Source libcamera based camera software stack point of view
> > > > > there doesn't seem to be a need for v4l2loopback or another similar driver.
> > > > > The two main reasons for this is that (1) there's no need for glueing
> > > > > something separate together like this and (2) V4L2 isn't a great
> > > > > application interface for cameras -- use libcamera or Pipewire instead.
> > > >
> > > > While I get this argument isolated, it does not match the observed
> > > > reality, and does not provide tools to address the core issue. I
> > > > will be in my grave before I've fixed the world like you are
> > > > suggesting :-)
> >
> > I really hope we'll provide a solution much faster than that :-)
> >
> > > > Like, first off, where would I use libcamera or Pipewire? There's
> > > > no well-defined target other than kernel in this problem.
> >
> > PipeWire is becoming the de facto media server on desktop systems, for
> > both audio and video. It has been shipped by distributions for a while
> > for audio, and is the core component that allows screen capture (and
> > therefore screen sharing in video conferencing) on Wayland-based
> > systems. For video, PipeWire support has most notably been integrated in
> > WebRTC, used by both Firefox and Chrome. The number of applications
> > using PipeWire is growing, OBS has recently received support for
> > PipeWire sources for instance. If you need to use it in an application
> > that requires a V4L2 capture device, the pw-v4l2 script emulates the
> > V4L2 API to provide a quick stopgap measure until applications get
> > native PipeWire support.
> >
> > libcamera solves an orthogonal problem, which is control of raw camera
> > sensors and ISPs typically found in mobile and embedded devices, and now
> > increasingly in laptops as well (Intel IPU3, IPU4, IPU6 and IPU7).
> > Applications typically don't use libcamera directly, but interface it
> > with GStreamer (libcamerasrc element) or PipeWire (which has native
> > libcamera support).
> >
> > While I understand that libcamera and PipeWire may be quite new for a
> > large number of users, the ecosystem is moving in that direction, and
> > both projects are very active.
>
> Thanks for the information and I take this into account when/if considering
> any updates. The response is so informative that I need to purge this a
> bit (thank you for that) :-) This does not disregard your response but
> personally I'm not have huge a fan of LD_PRELOAD style compatibility
> wrappers.
I'm not either as it can only provide best-effort compatibility, but it
has proven to be useful. For instance, we successfully tested the
libcamera LD_PRELOAD v4l2-compat.so with Firefox before support for
PipeWire was ready in WebRTC, providing a way to use ISPs in video
conferencing as a stopgap measure. It's all about helping the ecosystem
with the transition, and not intended as a long-term solution.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC PATCH v2] media: Virtual camera driver
From: Laurent Pinchart @ 2026-02-03 21:22 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Jani Nikula, johannes.goede, linux-media, anisse, oleksandr,
linux-integrity, Mauro Carvalho Chehab, Hans Verkuil,
Sakari Ailus, Jacopo Mondi, Ricardo Ribalda, open list
In-Reply-To: <aYJJWPLr4Qxkmn7z@kernel.org>
On Tue, Feb 03, 2026 at 09:15:36PM +0200, Jarkko Sakkinen wrote:
> On Tue, Feb 03, 2026 at 09:07:41PM +0200, Jarkko Sakkinen wrote:
> > On Tue, Feb 03, 2026 at 08:53:13PM +0200, Jani Nikula wrote:
> > > On Tue, 03 Feb 2026, johannes.goede@oss.qualcomm.com wrote:
> > > > OTHO evdi: https://github.com/DisplayLink/evdi has been kept out
> > > > of the kernel for pretty much the same reasons by the drm/kms folks.
> > > >
> > > > At least AFAIK there still is no way to present virtual kms capable
> > > > display outputs backed by userspace in the kernel.
> > > >
> > > > I completely understand where you're coming from wrt v4l2-loopback
> > > > support (or something equivalent) but asking for this really is
> > > > the same as asking for the evdi driver to get merged, which AFAIK
> > > > has been blocked for the reason of avoiding proprietary userspace
> > > > display output drivers (I guess there might be technical reasons too).
> > >
> > > I'm not sure this is the same thing, though.
> > >
> > > The DRM subsystem does require an open source userspace for new uAPI,
> > > which is stricter than most subsystems [1]. Other than that, I don't
> > > think anyone's actively keeping evdi out of the kernel. AFAIK there
> > > hasn't been a serious attempt at upstreaming it either. Which is pretty
> > > much because there's no open userspace. Nobody's cared enough to either
> > > write one or open source the existing one for a decade [2].
> >
> > This is unrelated of ACK/NACK and not saying this as a "selling point"
> > but realistically speaking based on what I've read I have extremely hard
> > time to believe that my driver would enable a market of proprietary
> > camera drivers :-) Actually, after looking up mipi.org based on Hans
> > response, I even more so believe that this is the case.
>
> With my maintainer hat on, I'd get the nack better if the driver was
> intrusive on changes to V4L2 subsystem itself. Then, it accumlates
> weight to other maintainers, and as we all have limited amount of time
> in our lives, I do get that. But since the driver would be compliance
> aligning leaf driver with a MAINTAINER entry of its own, it should not
> be a huge burden for V4L2 community and kernel maintainers. And I do
> have a track record of being long-term on maintaining stuff that I vomit
> out.
I don't dispute that. Even though we haven't really worked together on
kernel development, I have no prejudice towards your commitment as a
driver maintainer :-) There's nothing personal in this mail thread.
> That also does make difference as there are some guarantees that the
> end product would not be left into rotten.
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [RFC PATCH v2] media: Virtual camera driver
From: Jarkko Sakkinen @ 2026-02-03 21:40 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Jani Nikula, johannes.goede, linux-media, anisse, oleksandr,
linux-integrity, Mauro Carvalho Chehab, Hans Verkuil,
Sakari Ailus, Jacopo Mondi, Ricardo Ribalda, open list
In-Reply-To: <20260203212226.GF11369@killaraus>
On Tue, Feb 03, 2026 at 11:22:26PM +0200, Laurent Pinchart wrote:
> On Tue, Feb 03, 2026 at 09:15:36PM +0200, Jarkko Sakkinen wrote:
> > On Tue, Feb 03, 2026 at 09:07:41PM +0200, Jarkko Sakkinen wrote:
> > > On Tue, Feb 03, 2026 at 08:53:13PM +0200, Jani Nikula wrote:
> > > > On Tue, 03 Feb 2026, johannes.goede@oss.qualcomm.com wrote:
> > > > > OTHO evdi: https://github.com/DisplayLink/evdi has been kept out
> > > > > of the kernel for pretty much the same reasons by the drm/kms folks.
> > > > >
> > > > > At least AFAIK there still is no way to present virtual kms capable
> > > > > display outputs backed by userspace in the kernel.
> > > > >
> > > > > I completely understand where you're coming from wrt v4l2-loopback
> > > > > support (or something equivalent) but asking for this really is
> > > > > the same as asking for the evdi driver to get merged, which AFAIK
> > > > > has been blocked for the reason of avoiding proprietary userspace
> > > > > display output drivers (I guess there might be technical reasons too).
> > > >
> > > > I'm not sure this is the same thing, though.
> > > >
> > > > The DRM subsystem does require an open source userspace for new uAPI,
> > > > which is stricter than most subsystems [1]. Other than that, I don't
> > > > think anyone's actively keeping evdi out of the kernel. AFAIK there
> > > > hasn't been a serious attempt at upstreaming it either. Which is pretty
> > > > much because there's no open userspace. Nobody's cared enough to either
> > > > write one or open source the existing one for a decade [2].
> > >
> > > This is unrelated of ACK/NACK and not saying this as a "selling point"
> > > but realistically speaking based on what I've read I have extremely hard
> > > time to believe that my driver would enable a market of proprietary
> > > camera drivers :-) Actually, after looking up mipi.org based on Hans
> > > response, I even more so believe that this is the case.
> >
> > With my maintainer hat on, I'd get the nack better if the driver was
> > intrusive on changes to V4L2 subsystem itself. Then, it accumlates
> > weight to other maintainers, and as we all have limited amount of time
> > in our lives, I do get that. But since the driver would be compliance
> > aligning leaf driver with a MAINTAINER entry of its own, it should not
> > be a huge burden for V4L2 community and kernel maintainers. And I do
> > have a track record of being long-term on maintaining stuff that I vomit
> > out.
>
> I don't dispute that. Even though we haven't really worked together on
> kernel development, I have no prejudice towards your commitment as a
> driver maintainer :-) There's nothing personal in this mail thread.
Absolutely non taken either (and neither from my side).
>
> > That also does make difference as there are some guarantees that the
> > end product would not be left into rotten.
>
> --
> Regards,
>
> Laurent Pinchart
BR, Jarkko
^ permalink raw reply
* Re: [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs
From: Roberto Sassu @ 2026-02-04 12:36 UTC (permalink / raw)
To: Frederick Lawler, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
Darrick J. Wong, Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
In-Reply-To: <20260130-xfs-ima-fixup-v5-3-57e84ea91712@cloudflare.com>
On Fri, 2026-01-30 at 16:39 -0600, Frederick Lawler wrote:
> IMA performs unnecessary measurements on files in stacked file systems
> that do not set kstat.change_cookie to an inode's i_version.
>
> For example: TMPFS (upper) is stacked onto XFS (lower).
> Actions files result in re-measurement because commit 1cf7e834a6fb
> ("xfs: switch to multigrain timestamps") introduced multigrain
> timestamps into XFS which changed the kstat.change_cookie semantics
> to no longer supply an i_version to compare against in
> integrity_inode_attributes_changed(). Once the inode is in TMPFS,
> the change detection behavior operates as normal because TMPFS updates
> kstat.change_cookie to the i_version.
>
> Instead, fall back onto a ctime comparison. This also gives file systems
> that do not support i_version an opportunity avoid the same behavior,
> as they're more likely to have ctime support.
>
> timespec64_to_ns() is chosen to avoid adding extra storage to
> integrity_inode_attributes by leveraging the existing version field.
Correct me if I'm wrong, but this issue seems to me xfs-specific. In
that case, maybe it is better to remove the stacked filesystem part,
which might be misleading.
Thanks
Roberto
> Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> Suggested-by: Jeff Layton <jlayton@kernel.org>
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> ---
> include/linux/integrity.h | 6 +++++-
> security/integrity/ima/ima_api.c | 11 ++++++++---
> security/integrity/ima/ima_main.c | 2 +-
> 3 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index 382c783f0fa3ae4a938cdf9559291ba1903a378e..ec2c94907f417c4a71ecce29ac79edac9bc2c6f8 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -10,6 +10,7 @@
> #include <linux/fs.h>
> #include <linux/iversion.h>
> #include <linux/kernel.h>
> +#include <linux/time64.h>
>
> enum integrity_status {
> INTEGRITY_PASS = 0,
> @@ -58,6 +59,9 @@ integrity_inode_attrs_stat_changed
> if (stat->result_mask & STATX_CHANGE_COOKIE)
> return stat->change_cookie != attrs->version;
>
> + if (stat->result_mask & STATX_CTIME)
> + return timespec64_to_ns(&stat->ctime) != (s64)attrs->version;
> +
> return true;
> }
>
> @@ -84,7 +88,7 @@ integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> * only for IMA if vfs_getattr_nosec() fails.
> */
> if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> - STATX_CHANGE_COOKIE,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT))
> return !IS_I_VERSION(inode) ||
> !inode_eq_iversion(inode, attrs->version);
> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> index c35ea613c9f8d404ba4886e3b736c3bab29d1668..e47d6281febc15a0ac1bd2ea1d28fea4d0cd5c58 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -272,10 +272,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> * to an initial measurement/appraisal/audit, but was modified to
> * assume the file changed.
> */
> - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> + result = vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT);
> - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> - i_version = stat.change_cookie;
> + if (!result) {
> + if (stat.result_mask & STATX_CHANGE_COOKIE)
> + i_version = stat.change_cookie;
> + else if (stat.result_mask & STATX_CTIME)
> + i_version = timespec64_to_ns(&stat.ctime);
> + }
> hash.hdr.algo = algo;
> hash.hdr.length = hash_digest_size[algo];
>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 8cb17c9d446caaa5a98f5ec8f027c17ba7babca8..776db158b0bd8a0d053729ac0cc15af8b6020a98 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -199,7 +199,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> &iint->atomic_flags);
> if ((iint->flags & IMA_NEW_FILE) ||
> vfs_getattr_nosec(&file->f_path, &stat,
> - STATX_CHANGE_COOKIE,
> + STATX_CHANGE_COOKIE | STATX_CTIME,
> AT_STATX_SYNC_AS_STAT) ||
> integrity_inode_attrs_stat_changed(&iint->real_inode,
> &stat)) {
>
^ permalink raw reply
* Re: [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs
From: Roberto Sassu @ 2026-02-04 12:34 UTC (permalink / raw)
To: Frederick Lawler, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin,
Eric Snowberg, Paul Moore, James Morris, Serge E. Hallyn,
Darrick J. Wong, Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
In-Reply-To: <20260130-xfs-ima-fixup-v5-2-57e84ea91712@cloudflare.com>
On Fri, 2026-01-30 at 16:39 -0600, Frederick Lawler wrote:
> Align integrity_inode_attrs_changed() to ima_check_last_writer()'s
> semantics when detecting changes.
>
> For IMA, stacked file systems that do not set kstat.change_cookie,
> integrity_inode_attrs_changed() will compare zero to zero, thus no
I setup overlay with two xfs filesystems, kept the file I want to be
audited in the lower filesystem.
Without this patch set, if I modify the lower file, changes are
detected, because actually the i_version is incremented.
In which situation there is a comparison zero to zero?
Thanks
Roberto
> change detected. This is not dissimilar to what
> ima_check_last_writer() does.
>
> No logical change intended for EVM.
>
> Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> ---
> include/linux/integrity.h | 28 ++++++++++++++++++++++++----
> security/integrity/evm/evm_main.c | 5 ++---
> security/integrity/ima/ima_main.c | 5 ++---
> 3 files changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index beb9ab19fa6257e79266b58bcb5f55b0c5445828..382c783f0fa3ae4a938cdf9559291ba1903a378e 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -9,6 +9,7 @@
>
> #include <linux/fs.h>
> #include <linux/iversion.h>
> +#include <linux/kernel.h>
>
> enum integrity_status {
> INTEGRITY_PASS = 0,
> @@ -62,14 +63,33 @@ integrity_inode_attrs_stat_changed
>
> /*
> * On stacked filesystems detect whether the inode or its content has changed.
> + *
> + * Must be called in process context.
> */
> static inline bool
> integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> - const struct inode *inode)
> + struct file *file, struct inode *inode)
> {
> - return (inode->i_sb->s_dev != attrs->dev ||
> - inode->i_ino != attrs->ino ||
> - !inode_eq_iversion(inode, attrs->version));
> + struct kstat stat;
> +
> + might_sleep();
> +
> + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> + return true;
> +
> + /*
> + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> + * is not a good indicator of i_version support, this still retains
> + * the logic such that a re-evaluation should still occur for EVM, and
> + * only for IMA if vfs_getattr_nosec() fails.
> + */
> + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> + STATX_CHANGE_COOKIE,
> + AT_STATX_SYNC_AS_STAT))
> + return !IS_I_VERSION(inode) ||
> + !inode_eq_iversion(inode, attrs->version);
> +
> + return integrity_inode_attrs_stat_changed(attrs, &stat);
> }
>
>
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> bool ret = false;
>
> if (iint) {
> - ret = (!IS_I_VERSION(metadata_inode) ||
> - integrity_inode_attrs_changed(&iint->metadata_inode,
> - metadata_inode));
> + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> + NULL, metadata_inode);
> if (ret)
> iint->evm_status = INTEGRITY_UNKNOWN;
> }
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 6570ad10887b9ea1172c78274cf62482350e87ff..8cb17c9d446caaa5a98f5ec8f027c17ba7babca8 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -328,9 +328,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> real_inode = d_real_inode(file_dentry(file));
> if (real_inode != inode &&
> (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> - if (!IS_I_VERSION(real_inode) ||
> - integrity_inode_attrs_changed(&iint->real_inode,
> - real_inode)) {
> + if (integrity_inode_attrs_changed(&iint->real_inode,
> + file, real_inode)) {
> iint->flags &= ~IMA_DONE_MASK;
> iint->measured_pcrs = 0;
> }
>
^ permalink raw reply
* Re: [PATCH] evm: check return values of crypto_shash functions
From: Roberto Sassu @ 2026-02-04 12:50 UTC (permalink / raw)
To: Daniel Hodges, zohar, roberto.sassu
Cc: dmitry.kasatkin, eric.snowberg, paul, jmorris, serge,
linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <20260131182233.2365157-1-hodgesd@meta.com>
On Sat, 2026-01-31 at 10:22 -0800, Daniel Hodges wrote:
> The crypto_shash_update() and crypto_shash_final() functions can fail
> and return error codes, but their return values were being ignored in
> several places in evm_crypto.c:
>
> - hmac_add_misc(): ignores returns from crypto_shash_update() and
> crypto_shash_final()
> - evm_calc_hmac_or_hash(): ignores returns from crypto_shash_update()
> - evm_init_hmac(): ignores returns from crypto_shash_update()
>
> If these hash operations fail silently, the resulting HMAC could be
> invalid or incomplete. This could potentially allow integrity
> verification to pass with incorrect HMACs, weakening EVM's security
> guarantees.
>
> Fix this by:
> - Changing hmac_add_misc() from void to int return type
> - Checking and propagating error codes from all crypto_shash calls
> - Updating all callers to check the return values
>
> Fixes: 66dbc325afce ("evm: re-release")
> Signed-off-by: Daniel Hodges <hodgesd@meta.com>
> ---
> security/integrity/evm/evm_crypto.c | 45 +++++++++++++++++++----------
> 1 file changed, 30 insertions(+), 15 deletions(-)
>
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index a5e730ffda57..286f23a1a26b 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -132,58 +132,65 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
> }
> return desc;
> }
>
> /* Protect against 'cutting & pasting' security.evm xattr, include inode
> * specific info.
> *
> * (Additional directory/file metadata needs to be added for more complete
> * protection.)
> */
> -static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> - char type, char *digest)
> +static int hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> + char type, char *digest)
> {
> struct h_misc {
> unsigned long ino;
> __u32 generation;
> uid_t uid;
> gid_t gid;
> umode_t mode;
> } hmac_misc;
> + int ret;
>
> memset(&hmac_misc, 0, sizeof(hmac_misc));
> /* Don't include the inode or generation number in portable
> * signatures
> */
> if (type != EVM_XATTR_PORTABLE_DIGSIG) {
> hmac_misc.ino = inode->i_ino;
> hmac_misc.generation = inode->i_generation;
> }
> /* The hmac uid and gid must be encoded in the initial user
> * namespace (not the filesystems user namespace) as encoding
> * them in the filesystems user namespace allows an attack
> * where first they are written in an unprivileged fuse mount
> * of a filesystem and then the system is tricked to mount the
> * filesystem for real on next boot and trust it because
> * everything is signed.
> */
> hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
> hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
> hmac_misc.mode = inode->i_mode;
> - crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
> + ret = crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
> + if (ret)
> + return ret;
> if ((evm_hmac_attrs & EVM_ATTR_FSUUID) &&
> - type != EVM_XATTR_PORTABLE_DIGSIG)
> - crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
> - crypto_shash_final(desc, digest);
> + type != EVM_XATTR_PORTABLE_DIGSIG) {
> + ret = crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
> + if (ret)
> + return ret;
> + }
> + ret = crypto_shash_final(desc, digest);
Maybe we should also indicate if an error occurred, with a separate
error message, or adding the result in the message below.
Thanks
Roberto
> pr_debug("hmac_misc: (%zu) [%*phN]\n", sizeof(struct h_misc),
> (int)sizeof(struct h_misc), &hmac_misc);
> + return ret;
> }
>
> /*
> * Dump large security xattr values as a continuous ascii hexadecimal string.
> * (pr_debug is limited to 64 bytes.)
> */
> static void dump_security_xattr_l(const char *prefix, const void *src,
> size_t count)
> {
> #if defined(DEBUG) || defined(CONFIG_DYNAMIC_DEBUG)
> @@ -253,23 +260,24 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>
> /*
> * Skip non-enabled xattrs for locally calculated
> * signatures/HMACs.
> */
> if (type != EVM_XATTR_PORTABLE_DIGSIG && !xattr->enabled)
> continue;
>
> if ((req_xattr_name && req_xattr_value)
> && !strcmp(xattr->name, req_xattr_name)) {
> - error = 0;
> - crypto_shash_update(desc, (const u8 *)req_xattr_value,
> - req_xattr_value_len);
> + error = crypto_shash_update(desc, (const u8 *)req_xattr_value,
> + req_xattr_value_len);
> + if (error)
> + goto out;
> if (is_ima)
> ima_present = true;
>
> dump_security_xattr(req_xattr_name,
> req_xattr_value,
> req_xattr_value_len);
> continue;
> }
> size = vfs_getxattr_alloc(&nop_mnt_idmap, dentry, xattr->name,
> &xattr_value, xattr_size, GFP_NOFS);
> @@ -279,29 +287,32 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> }
> if (size < 0)
> continue;
>
> user_space_size = vfs_getxattr(&nop_mnt_idmap, dentry,
> xattr->name, NULL, 0);
> if (user_space_size != size)
> pr_debug("file %s: xattr %s size mismatch (kernel: %d, user: %d)\n",
> dentry->d_name.name, xattr->name, size,
> user_space_size);
> - error = 0;
> xattr_size = size;
> - crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
> + error = crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
> + if (error)
> + goto out;
> if (is_ima)
> ima_present = true;
>
> dump_security_xattr(xattr->name, xattr_value, xattr_size);
> }
> - hmac_add_misc(desc, inode, type, data->digest);
> + error = hmac_add_misc(desc, inode, type, data->digest);
> + if (error)
> + goto out;
>
> if (inode != d_backing_inode(dentry) && iint) {
> if (IS_I_VERSION(inode))
> i_version = inode_query_iversion(inode);
> integrity_inode_attrs_store(&iint->metadata_inode, i_version,
> inode);
> }
>
> /* Portable EVM signatures must include an IMA hash */
> if (type == EVM_XATTR_PORTABLE_DIGSIG && !ima_present)
> @@ -394,37 +405,41 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
> rc = __vfs_removexattr(&nop_mnt_idmap, dentry, XATTR_NAME_EVM);
> }
> return rc;
> }
>
> int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
> char *hmac_val)
> {
> struct shash_desc *desc;
> const struct xattr *xattr;
> + int ret;
>
> desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
> if (IS_ERR(desc)) {
> pr_info("init_desc failed\n");
> return PTR_ERR(desc);
> }
>
> for (xattr = xattrs; xattr->name; xattr++) {
> if (!evm_protected_xattr(xattr->name))
> continue;
>
> - crypto_shash_update(desc, xattr->value, xattr->value_len);
> + ret = crypto_shash_update(desc, xattr->value, xattr->value_len);
> + if (ret)
> + goto out;
> }
>
> - hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
> + ret = hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
> +out:
> kfree(desc);
> - return 0;
> + return ret;
> }
>
> /*
> * Get the key from the TPM for the SHA1-HMAC
> */
> int evm_init_key(void)
> {
> struct key *evm_key;
> struct encrypted_key_payload *ekp;
> int rc;
^ permalink raw reply
* Re: [PATCH] evm: check return values of crypto_shash functions
From: Daniel Hodges @ 2026-02-04 15:47 UTC (permalink / raw)
To: Roberto Sassu
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge, linux-integrity, linux-security-module,
linux-kernel
In-Reply-To: <d1d63c522a3842ccaf74c4462ee06bf82ce3b3f5.camel@huaweicloud.com>
On Wed, Feb 04, 2026 at 01:50:29PM +0100, Roberto Sassu wrote:
> On Sat, 2026-01-31 at 10:22 -0800, Daniel Hodges wrote:
> > The crypto_shash_update() and crypto_shash_final() functions can fail
> > and return error codes, but their return values were being ignored in
> > several places in evm_crypto.c:
> >
> > - hmac_add_misc(): ignores returns from crypto_shash_update() and
> > crypto_shash_final()
> > - evm_calc_hmac_or_hash(): ignores returns from crypto_shash_update()
> > - evm_init_hmac(): ignores returns from crypto_shash_update()
> >
> > If these hash operations fail silently, the resulting HMAC could be
> > invalid or incomplete. This could potentially allow integrity
> > verification to pass with incorrect HMACs, weakening EVM's security
> > guarantees.
> >
> > Fix this by:
> > - Changing hmac_add_misc() from void to int return type
> > - Checking and propagating error codes from all crypto_shash calls
> > - Updating all callers to check the return values
> >
> > Fixes: 66dbc325afce ("evm: re-release")
> > Signed-off-by: Daniel Hodges <hodgesd@meta.com>
> > ---
> > security/integrity/evm/evm_crypto.c | 45 +++++++++++++++++++----------
> > 1 file changed, 30 insertions(+), 15 deletions(-)
> >
> > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > index a5e730ffda57..286f23a1a26b 100644
> > --- a/security/integrity/evm/evm_crypto.c
> > +++ b/security/integrity/evm/evm_crypto.c
> > @@ -132,58 +132,65 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
> > }
> > return desc;
> > }
> >
> > /* Protect against 'cutting & pasting' security.evm xattr, include inode
> > * specific info.
> > *
> > * (Additional directory/file metadata needs to be added for more complete
> > * protection.)
> > */
> > -static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> > - char type, char *digest)
> > +static int hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> > + char type, char *digest)
> > {
> > struct h_misc {
> > unsigned long ino;
> > __u32 generation;
> > uid_t uid;
> > gid_t gid;
> > umode_t mode;
> > } hmac_misc;
> > + int ret;
> >
> > memset(&hmac_misc, 0, sizeof(hmac_misc));
> > /* Don't include the inode or generation number in portable
> > * signatures
> > */
> > if (type != EVM_XATTR_PORTABLE_DIGSIG) {
> > hmac_misc.ino = inode->i_ino;
> > hmac_misc.generation = inode->i_generation;
> > }
> > /* The hmac uid and gid must be encoded in the initial user
> > * namespace (not the filesystems user namespace) as encoding
> > * them in the filesystems user namespace allows an attack
> > * where first they are written in an unprivileged fuse mount
> > * of a filesystem and then the system is tricked to mount the
> > * filesystem for real on next boot and trust it because
> > * everything is signed.
> > */
> > hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
> > hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
> > hmac_misc.mode = inode->i_mode;
> > - crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
> > + ret = crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
> > + if (ret)
> > + return ret;
> > if ((evm_hmac_attrs & EVM_ATTR_FSUUID) &&
> > - type != EVM_XATTR_PORTABLE_DIGSIG)
> > - crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
> > - crypto_shash_final(desc, digest);
> > + type != EVM_XATTR_PORTABLE_DIGSIG) {
> > + ret = crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
> > + if (ret)
> > + return ret;
> > + }
> > + ret = crypto_shash_final(desc, digest);
>
> Maybe we should also indicate if an error occurred, with a separate
> error message, or adding the result in the message below.
>
> Thanks
>
> Roberto
That makes sense, I'll send a V2. I'm having trouble with my corporate
email mail delivery so it might come from my personal email.
^ permalink raw reply
* Re: [PATCH v15 02/28] tpm: Move TPM1 specific definitions and functions to new headers
From: ross.philipson @ 2026-02-04 17:44 UTC (permalink / raw)
To: Daniel P. Smith, Jarkko Sakkinen
Cc: linux-kernel, x86, linux-integrity, linux-doc, linux-crypto,
kexec, linux-efi, iommu, tglx, mingo, bp, hpa, dave.hansen, ardb,
mjg59, James.Bottomley, peterhuewe, jgg, luto, nivedita, herbert,
davem, corbet, ebiederm, dwmw2, baolu.lu, kanth.ghatraju,
andrew.cooper3, trenchboot-devel
In-Reply-To: <7aad3461-f0a5-42ff-ba9b-b52edd7d36db@apertussolutions.com>
On 2/1/26 8:23 AM, Daniel P. Smith wrote:
> On 1/19/26 18:57, Jarkko Sakkinen wrote:
>> On Mon, Dec 15, 2025 at 03:32:50PM -0800, Ross Philipson wrote:
>>> This gathers all the TPM1 definitions and structures into two separate
>>> header files (public tpm1.h and private tpm1_structs.h). The definitions
>>> moved to these files correspond to the TCG specification for TPM 1 family:
>>>
>>> TPM 1.2 Main Specification
>>> - https://urldefense.com/v3/__https://trustedcomputinggroup.org/resource/tpm-main-specification/__;!!ACWV5N9M2RV99hQ!OivTxO-R0nvBJYXIV-T0n0fq0wY64MTgdaecPzAauxrFkRxTcQ6CuBEcXmZZYY1KfZlqmfLTZBbPaEr7Y8k7hJk9_mQ$
>>> Note that the structures were pulled into tpm1_structs.h to allow their
>>> external reuse.
>>>
>>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>>> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
>>> ---
>>> drivers/char/tpm/tpm.h | 98 +--------------------------------
>>> drivers/char/tpm/tpm1-cmd.c | 5 --
>>> drivers/char/tpm/tpm1_structs.h | 97 ++++++++++++++++++++++++++++++++
>>
>> I think you are overcomplicating the patch set and doing more
>> than you really need to do.
>>
>> I.e. structs could go also just as well to tpm_command.h. We
>> will deal with if/when that file ever grows too large. It's
>> absolutely not a priority for this patch set.
>
> Ack.
>
>>> include/linux/tpm1.h | 34 +++++++++++-
>>> 4 files changed, 132 insertions(+), 102 deletions(-)
>>> create mode 100644 drivers/char/tpm/tpm1_structs.h
>>>
>>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>>> index ca391b2a211c..1f9f8540eede 100644
>>> --- a/drivers/char/tpm/tpm.h
>>> +++ b/drivers/char/tpm/tpm.h
>>> @@ -50,105 +50,9 @@ enum tpm_addr {
>>> TPM_ADDR = 0x4E,
>>> };
>>> -#define TPM_WARN_RETRY 0x800
>>> -#define TPM_WARN_DOING_SELFTEST 0x802
>>> -#define TPM_ERR_DEACTIVATED 0x6
>>> -#define TPM_ERR_DISABLED 0x7
>>> -#define TPM_ERR_FAILEDSELFTEST 0x1C
>>> -#define TPM_ERR_INVALID_POSTINIT 38
>>> -
>>> -#define TPM_TAG_RQU_COMMAND 193
>>> -
>>> /* TPM2 specific constants. */
>>> #define TPM2_SPACE_BUFFER_SIZE 16384 /* 16 kB */
>>> -struct stclear_flags_t {
>>> - __be16 tag;
>>> - u8 deactivated;
>>> - u8 disableForceClear;
>>> - u8 physicalPresence;
>>> - u8 physicalPresenceLock;
>>> - u8 bGlobalLock;
>>> -} __packed;
>>> -
>>> -struct tpm1_version {
>>> - u8 major;
>>> - u8 minor;
>>> - u8 rev_major;
>>> - u8 rev_minor;
>>> -} __packed;
>>> -
>>> -struct tpm1_version2 {
>>> - __be16 tag;
>>> - struct tpm1_version version;
>>> -} __packed;
>>> -
>>> -struct timeout_t {
>>> - __be32 a;
>>> - __be32 b;
>>> - __be32 c;
>>> - __be32 d;
>>> -} __packed;
>>> -
>>> -struct duration_t {
>>> - __be32 tpm_short;
>>> - __be32 tpm_medium;
>>> - __be32 tpm_long;
>>> -} __packed;
>>> -
>>> -struct permanent_flags_t {
>>> - __be16 tag;
>>> - u8 disable;
>>> - u8 ownership;
>>> - u8 deactivated;
>>> - u8 readPubek;
>>> - u8 disableOwnerClear;
>>> - u8 allowMaintenance;
>>> - u8 physicalPresenceLifetimeLock;
>>> - u8 physicalPresenceHWEnable;
>>> - u8 physicalPresenceCMDEnable;
>>> - u8 CEKPUsed;
>>> - u8 TPMpost;
>>> - u8 TPMpostLock;
>>> - u8 FIPS;
>>> - u8 operator;
>>> - u8 enableRevokeEK;
>>> - u8 nvLocked;
>>> - u8 readSRKPub;
>>> - u8 tpmEstablished;
>>> - u8 maintenanceDone;
>>> - u8 disableFullDALogicInfo;
>>> -} __packed;
>>> -
>>> -typedef union {
>>> - struct permanent_flags_t perm_flags;
>>> - struct stclear_flags_t stclear_flags;
>>> - __u8 owned;
>>> - __be32 num_pcrs;
>>> - struct tpm1_version version1;
>>> - struct tpm1_version2 version2;
>>> - __be32 manufacturer_id;
>>> - struct timeout_t timeout;
>>> - struct duration_t duration;
>>> -} cap_t;
>>> -
>>> -enum tpm_capabilities {
>>> - TPM_CAP_FLAG = 4,
>>> - TPM_CAP_PROP = 5,
>>> - TPM_CAP_VERSION_1_1 = 0x06,
>>> - TPM_CAP_VERSION_1_2 = 0x1A,
>>> -};
>>> -
>>> -enum tpm_sub_capabilities {
>>> - TPM_CAP_PROP_PCR = 0x101,
>>> - TPM_CAP_PROP_MANUFACTURER = 0x103,
>>> - TPM_CAP_FLAG_PERM = 0x108,
>>> - TPM_CAP_FLAG_VOL = 0x109,
>>> - TPM_CAP_PROP_OWNER = 0x111,
>>> - TPM_CAP_PROP_TIS_TIMEOUT = 0x115,
>>> - TPM_CAP_PROP_TIS_DURATION = 0x120,
>>> -};
>>> -
>>> enum tpm2_pt_props {
>>> TPM2_PT_NONE = 0x00000000,
>>> TPM2_PT_GROUP = 0x00000100,
>>> @@ -229,6 +133,8 @@ enum tpm2_pt_props {
>>> * compiler warnings about stack frame size. */
>>> #define TPM_MAX_RNG_DATA 128
>>> +#include "tpm1_structs.h"
>>> +
>>> extern const struct class tpm_class;
>>> extern const struct class tpmrm_class;
>>> extern dev_t tpm_devt;
>>> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
>>> index f29827b454d2..02f20a0aa37d 100644
>>> --- a/drivers/char/tpm/tpm1-cmd.c
>>> +++ b/drivers/char/tpm/tpm1-cmd.c
>>> @@ -505,11 +505,6 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>>> }
>>> EXPORT_SYMBOL_GPL(tpm1_getcap);
>>> -struct tpm1_get_random_out {
>>> - __be32 rng_data_len;
>>> - u8 rng_data[TPM_MAX_RNG_DATA];
>>> -} __packed;
>>> -
>>> /**
>>> * tpm1_get_random() - get random bytes from the TPM's RNG
>>> * @chip: a &struct tpm_chip instance
>>> diff --git a/drivers/char/tpm/tpm1_structs.h b/drivers/char/tpm/tpm1_structs.h
>>> new file mode 100644
>>> index 000000000000..ad21376af5ab
>>> --- /dev/null
>>> +++ b/drivers/char/tpm/tpm1_structs.h
>>> @@ -0,0 +1,97 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Copyright (C) 2004 IBM Corporation
>>> + * Copyright (C) 2015 Intel Corporation
>>> + *
>>> + * Authors:
>>> + * Leendert van Doorn <leendert@watson.ibm.com>
>>> + * Dave Safford <safford@watson.ibm.com>
>>> + * Reiner Sailer <sailer@watson.ibm.com>
>>> + * Kylene Hall <kjhall@us.ibm.com>
>>> + *
>>> + * Maintained by: <tpmdd-devel@lists.sourceforge.net>
>>> + *
>>> + * Device driver for TCG/TCPA TPM (trusted platform module).
>>> + * Specifications at https://urldefense.com/v3/__http://www.trustedcomputinggroup.org__;!!ACWV5N9M2RV99hQ!OivTxO-R0nvBJYXIV-T0n0fq0wY64MTgdaecPzAauxrFkRxTcQ6CuBEcXmZZYY1KfZlqmfLTZBbPaEr7Y8k7HW5iz_w$ + */
>>> +
>>> +#ifndef __TPM1_STRUCTS_H__
>>> +#define __TPM1_STRUCTS_H__
>>> +
>>> +struct stclear_flags_t {
>>> + __be16 tag;
>>> + u8 deactivated;
>>> + u8 disableForceClear;
>>> + u8 physicalPresence;
>>> + u8 physicalPresenceLock;
>>> + u8 bGlobalLock;
>>> +} __packed;
>>
>>
>> Don't retain alignment.
>>
>
> Okay.
I wanted to clarify what you are asking here. Do you mean to just not pack the structs? It seems some of these structs define the responses from the TPM chip and I would assume they are packed this way.
>
>>> +
>>> +struct tpm1_version {
>>> + u8 major;
>>> + u8 minor;
>>> + u8 rev_major;
>>> + u8 rev_minor;
>>> +} __packed;
>>> +
>>> +struct tpm1_version2 {
>>> + __be16 tag;
>>> + struct tpm1_version version;
>>> +} __packed;
>>> +
>>> +struct timeout_t {
>>> + __be32 a;
>>> + __be32 b;
>>> + __be32 c;
>>> + __be32 d;
>>> +} __packed;
>>> +
>>> +struct duration_t {
>>> + __be32 tpm_short;
>>> + __be32 tpm_medium;
>>> + __be32 tpm_long;
>>> +} __packed;
>>> +
>>> +struct permanent_flags_t {
>>> + __be16 tag;
>>> + u8 disable;
>>> + u8 ownership;
>>> + u8 deactivated;
>>> + u8 readPubek;
>>> + u8 disableOwnerClear;
>>> + u8 allowMaintenance;
>>> + u8 physicalPresenceLifetimeLock;
>>> + u8 physicalPresenceHWEnable;
>>> + u8 physicalPresenceCMDEnable;
>>> + u8 CEKPUsed;
>>> + u8 TPMpost;
>>> + u8 TPMpostLock;
>>> + u8 FIPS;
>>> + u8 operator;
>>> + u8 enableRevokeEK;
>>> + u8 nvLocked;
>>> + u8 readSRKPub;
>>> + u8 tpmEstablished;
>>> + u8 maintenanceDone;
>>> + u8 disableFullDALogicInfo;
>>> +} __packed;
>>> +
>>> +/* Gather all capabilities related information info one type */
>>> +typedef union {
>>> + struct permanent_flags_t perm_flags;
>>> + struct stclear_flags_t stclear_flags;
>>> + __u8 owned;
>>> + __be32 num_pcrs;
>>> + struct tpm1_version version1;
>>> + struct tpm1_version2 version2;
>>> + __be32 manufacturer_id;
>>> + struct timeout_t timeout;
>>> + struct duration_t duration;
>>> +} cap_t;
>>
>> Don't retain alignment here.
Again to clarify, do you mean none of these structs should be packed (and thus the union won't be either)?
Thanks
Ross
>>
>
> Okay.
>
>>> +
>>> +struct tpm1_get_random_out {
>>> + __be32 rng_data_len;
>>> + u8 rng_data[TPM_MAX_RNG_DATA];
>>> +} __packed;
>>> +
>>> +#endif
>>> diff --git a/include/linux/tpm1.h b/include/linux/tpm1.h
>>> index 54c6c211eb9e..5fad94ac8d15 100644
>>> --- a/include/linux/tpm1.h
>>> +++ b/include/linux/tpm1.h
>>> @@ -47,7 +47,39 @@ enum tpm_command_ordinals {
>>> TPM_ORD_UNSEAL = 24,
>>> };
>>> -/* Other constants */
>>> +enum tpm_capabilities {
>>> + TPM_CAP_FLAG = 4,
>>> + TPM_CAP_PROP = 5,
>>> + TPM_CAP_VERSION_1_1 = 0x06,
>>> + TPM_CAP_VERSION_1_2 = 0x1A,
>>> +};
>>> +
>>> +enum tpm_sub_capabilities {
>>> + TPM_CAP_PROP_PCR = 0x101,
>>> + TPM_CAP_PROP_MANUFACTURER = 0x103,
>>> + TPM_CAP_FLAG_PERM = 0x108,
>>> + TPM_CAP_FLAG_VOL = 0x109,
>>> + TPM_CAP_PROP_OWNER = 0x111,
>>> + TPM_CAP_PROP_TIS_TIMEOUT = 0x115,
>>> + TPM_CAP_PROP_TIS_DURATION = 0x120,
>>> +};
>>> +
>>> +/* Return Codes */
>>> +enum tpm_return_codes {
>>> + TPM_BASE_MASK = 0,
>>> + TPM_NON_FATAL_MASK = 0x00000800,
>>> + TPM_SUCCESS = TPM_BASE_MASK + 0,
>>> + TPM_ERR_DEACTIVATED = TPM_BASE_MASK + 6,
>>> + TPM_ERR_DISABLED = TPM_BASE_MASK + 7,
>>> + TPM_ERR_FAIL = TPM_BASE_MASK + 9,
>>> + TPM_ERR_FAILEDSELFTEST = TPM_BASE_MASK + 28,
>>> + TPM_ERR_INVALID_POSTINIT = TPM_BASE_MASK + 38,
>>> + TPM_ERR_INVALID_FAMILY = TPM_BASE_MASK + 55,
>>> + TPM_WARN_RETRY = TPM_BASE_MASK + TPM_NON_FATAL_MASK + 0,
>>> + TPM_WARN_DOING_SELFTEST = TPM_BASE_MASK + TPM_NON_FATAL_MASK + 2,
>>> +};
>>> +
>>> +/* Misc. constants */
>>
>> These constants should be relocated in a separate patch.
>>
>
> Okay.
>
>>> #define SRKHANDLE 0x40000000
>>> #define TPM_NONCE_SIZE 20
>>> #define TPM_ST_CLEAR 1
>>> --
>>> 2.43.7
>>>
>>
>> BR, Jarkko
>
^ permalink raw reply
* Re: [PATCH v4] ima: Fallback to ctime check for FS without kstat.change_cookie
From: Mimi Zohar @ 2026-02-04 19:32 UTC (permalink / raw)
To: Frederick Lawler, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton
Cc: linux-kernel, linux-integrity, linux-security-module, kernel-team
In-Reply-To: <1da265acd3168a5c2b7390ddafc6a46d5e2d5613.camel@linux.ibm.com>
Hi Fred,
On Fri, 2026-01-30 at 08:00 -0500, Mimi Zohar wrote:
> > Examples are fine, but first describe the problem - not detecting file change on
> > xfs.
> >
> > >
> > > In the case of stacking XFS on XFS, an action on either the LOWER or UPPER
> > > will require re-evaluation. Stacking TMPFS on XFS for instance, once the
> > > inode is UPPER is mutated, IMA resumes normal behavior because TMPFS
> > > leverages generic_fillattr() to update the change cookie.
> >
> > This sounds like the same issue - not detecting file change on xfs. The problem
> > is simply manifesting itself on stacked filesystems.
>
> Splitting this patch, so that the base XFS changes are in one patch and the
> stacked filesystem changes are in the other, would really help clarify what is
> needed and the reason why.
Tweaking your script with the following changes, you'll see duplicate audit
entries on the base XFS system, without requiring an overlay filesystem.
echo "audit func=FILE_CHECK uid=$(id -u nobody)" > "$IMA_POLICY"
setpriv --reuid nobody $PWD/rdwr "$TEST_FILE"
setpriv --reuid nobody $PWD/rdwr "$TEST_FILE"
audit_count=$(grep -e "file=\"$TEST_FILE\"" /var/log/audit/audit.log | grep -c
"comm=\"rdwr\"")
"rdwr" is a C program that opens the file read-write, using the open() syscall.
Mimi
^ permalink raw reply
* Re: [PATCH v5 2/3] ima: Make integrity_inode_attrs_changed() call into vfs
From: Frederick Lawler @ 2026-02-04 19:35 UTC (permalink / raw)
To: Roberto Sassu
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton, linux-kernel,
linux-integrity, linux-security-module, kernel-team
In-Reply-To: <9c25c201f384d7320fd83e648d61a0086016ee36.camel@huaweicloud.com>
On Wed, Feb 04, 2026 at 01:34:09PM +0100, Roberto Sassu wrote:
> On Fri, 2026-01-30 at 16:39 -0600, Frederick Lawler wrote:
> > Align integrity_inode_attrs_changed() to ima_check_last_writer()'s
> > semantics when detecting changes.
> >
> > For IMA, stacked file systems that do not set kstat.change_cookie,
> > integrity_inode_attrs_changed() will compare zero to zero, thus no
>
> I setup overlay with two xfs filesystems, kept the file I want to be
> audited in the lower filesystem.
>
> Without this patch set, if I modify the lower file, changes are
> detected, because actually the i_version is incremented.
Correct, but the test example in 00/03 demonstrates that there's no
modification. For workloads that only execute and not modify,
IMA shouldn't evaluate more than once, but that's what we're
observing at least for XFS.
>
> In which situation there is a comparison zero to zero?
My mistake. You're right, but for the wrong reason.
To be clear, these patches are about the STATX_CHANGE_COOKIE mechanic.
XFS updates the i_version regardless of the multigrain ctime changes.
You're correct in that with/without this patch there is no zero-zero
comparison for XFS, and that's because XFS isn't setting the
STATX_CHANGE_COOKIE in the result mask either for last writer check or
the attrs changed check, thus a change is always detected with
integrity_inode_attrs_stat_changed(), and thus maintains current
IMA behavior for XFS.
That said, should a file system set STATX_CHANGE_COOKIE in the result mask,
and not update the i_version (say its kept at zero), then it's always
zero-zero. I don't know how likely that scenario is.
I should reword this commit, but I am a bit hesitant to say "don't
squash this in with patch 3" due to that uncertainty.
>
> Thanks
>
> Roberto
>
> > change detected. This is not dissimilar to what
> > ima_check_last_writer() does.
> >
> > No logical change intended for EVM.
> >
> > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > ---
> > include/linux/integrity.h | 28 ++++++++++++++++++++++++----
> > security/integrity/evm/evm_main.c | 5 ++---
> > security/integrity/ima/ima_main.c | 5 ++---
> > 3 files changed, 28 insertions(+), 10 deletions(-)
> >
> > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > index beb9ab19fa6257e79266b58bcb5f55b0c5445828..382c783f0fa3ae4a938cdf9559291ba1903a378e 100644
> > --- a/include/linux/integrity.h
> > +++ b/include/linux/integrity.h
> > @@ -9,6 +9,7 @@
> >
> > #include <linux/fs.h>
> > #include <linux/iversion.h>
> > +#include <linux/kernel.h>
> >
> > enum integrity_status {
> > INTEGRITY_PASS = 0,
> > @@ -62,14 +63,33 @@ integrity_inode_attrs_stat_changed
> >
> > /*
> > * On stacked filesystems detect whether the inode or its content has changed.
> > + *
> > + * Must be called in process context.
> > */
> > static inline bool
> > integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > - const struct inode *inode)
> > + struct file *file, struct inode *inode)
> > {
> > - return (inode->i_sb->s_dev != attrs->dev ||
> > - inode->i_ino != attrs->ino ||
> > - !inode_eq_iversion(inode, attrs->version));
> > + struct kstat stat;
> > +
> > + might_sleep();
> > +
> > + if (inode->i_sb->s_dev != attrs->dev || inode->i_ino != attrs->ino)
> > + return true;
> > +
> > + /*
> > + * EVM currently relies on backing inode i_version. While IS_I_VERSION
> > + * is not a good indicator of i_version support, this still retains
> > + * the logic such that a re-evaluation should still occur for EVM, and
> > + * only for IMA if vfs_getattr_nosec() fails.
> > + */
> > + if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE,
> > + AT_STATX_SYNC_AS_STAT))
> > + return !IS_I_VERSION(inode) ||
> > + !inode_eq_iversion(inode, attrs->version);
> > +
> > + return integrity_inode_attrs_stat_changed(attrs, &stat);
> > }
> >
> >
> > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> > index 73d500a375cb37a54f295b0e1e93fd6e5d9ecddc..6a4e0e246005246d5700b1db590c1759242b9cb6 100644
> > --- a/security/integrity/evm/evm_main.c
> > +++ b/security/integrity/evm/evm_main.c
> > @@ -752,9 +752,8 @@ bool evm_metadata_changed(struct inode *inode, struct inode *metadata_inode)
> > bool ret = false;
> >
> > if (iint) {
> > - ret = (!IS_I_VERSION(metadata_inode) ||
> > - integrity_inode_attrs_changed(&iint->metadata_inode,
> > - metadata_inode));
> > + ret = integrity_inode_attrs_changed(&iint->metadata_inode,
> > + NULL, metadata_inode);
> > if (ret)
> > iint->evm_status = INTEGRITY_UNKNOWN;
> > }
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 6570ad10887b9ea1172c78274cf62482350e87ff..8cb17c9d446caaa5a98f5ec8f027c17ba7babca8 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -328,9 +328,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
> > real_inode = d_real_inode(file_dentry(file));
> > if (real_inode != inode &&
> > (action & IMA_DO_MASK) && (iint->flags & IMA_DONE_MASK)) {
> > - if (!IS_I_VERSION(real_inode) ||
> > - integrity_inode_attrs_changed(&iint->real_inode,
> > - real_inode)) {
> > + if (integrity_inode_attrs_changed(&iint->real_inode,
> > + file, real_inode)) {
> > iint->flags &= ~IMA_DONE_MASK;
> > iint->measured_pcrs = 0;
> > }
> >
>
^ permalink raw reply
* Re: [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs
From: Frederick Lawler @ 2026-02-04 21:22 UTC (permalink / raw)
To: Roberto Sassu
Cc: Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Darrick J. Wong,
Christian Brauner, Josef Bacik, Jeff Layton, linux-kernel,
linux-integrity, linux-security-module, kernel-team
In-Reply-To: <c449523aef301a6b199e06d4c3fbf7587d1218c5.camel@huaweicloud.com>
On Wed, Feb 04, 2026 at 01:36:09PM +0100, Roberto Sassu wrote:
> On Fri, 2026-01-30 at 16:39 -0600, Frederick Lawler wrote:
> > IMA performs unnecessary measurements on files in stacked file systems
> > that do not set kstat.change_cookie to an inode's i_version.
> >
> > For example: TMPFS (upper) is stacked onto XFS (lower).
> > Actions files result in re-measurement because commit 1cf7e834a6fb
> > ("xfs: switch to multigrain timestamps") introduced multigrain
> > timestamps into XFS which changed the kstat.change_cookie semantics
> > to no longer supply an i_version to compare against in
> > integrity_inode_attributes_changed(). Once the inode is in TMPFS,
> > the change detection behavior operates as normal because TMPFS updates
> > kstat.change_cookie to the i_version.
> >
> > Instead, fall back onto a ctime comparison. This also gives file systems
> > that do not support i_version an opportunity avoid the same behavior,
> > as they're more likely to have ctime support.
> >
> > timespec64_to_ns() is chosen to avoid adding extra storage to
> > integrity_inode_attributes by leveraging the existing version field.
>
> Correct me if I'm wrong, but this issue seems to me xfs-specific. In
> that case, maybe it is better to remove the stacked filesystem part,
> which might be misleading.
I'm using XFS because that's a clear example, but as Jeff pointed out in
an earlier email, there's too many file systems to account for. It's
clear from Jeff's prior email[1] that the intention is to stop using
change cookie for change detection for more file systems in favor
of multigrain ctime.
The stacked part is important because AFAIK, if not on stacked file system
the check in process_measurement() line 329 is skipped because
of the last_writer check doing the atomic read on the write count.
That said, I think Mimi pointed out in an email [2] where multi-grain
file systems are impacted regardless of stacked fs or not due to the last
writer check.
I don't recall coming across that in my tests, but perhaps I did that
specific test wrong? To be sure, I created the C program, and on the VM,
created a XFS disk, mounted it on loopback, ran the rdwr program on
"somefile" multiple times, and only got 1 audit log for it, until I
mutated it with touch, and only got 2 hits: original + after mutation
after running the program multiple times.
I'm not sure what's going on there, so I'll look into that a bit more,
but so far the impact is stacked file systems & multigrain ctime AFAIK.
[1]: https://lore.kernel.org/all/75618d9f454aece9a991d74eaf7ae5160828e901.camel@kernel.org/
[2]: https://lore.kernel.org/all/69540ac3aca536db948b6585b7076946a12ebe36.camel@linux.ibm.com/
>
> Thanks
>
> Roberto
>
> > Link: https://lore.kernel.org/all/aTspr4_h9IU4EyrR@CMGLRV3
> > Fixes: 1cf7e834a6fb ("xfs: switch to multigrain timestamps")
> > Suggested-by: Jeff Layton <jlayton@kernel.org>
> > Signed-off-by: Frederick Lawler <fred@cloudflare.com>
> > ---
> > include/linux/integrity.h | 6 +++++-
> > security/integrity/ima/ima_api.c | 11 ++++++++---
> > security/integrity/ima/ima_main.c | 2 +-
> > 3 files changed, 14 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> > index 382c783f0fa3ae4a938cdf9559291ba1903a378e..ec2c94907f417c4a71ecce29ac79edac9bc2c6f8 100644
> > --- a/include/linux/integrity.h
> > +++ b/include/linux/integrity.h
> > @@ -10,6 +10,7 @@
> > #include <linux/fs.h>
> > #include <linux/iversion.h>
> > #include <linux/kernel.h>
> > +#include <linux/time64.h>
> >
> > enum integrity_status {
> > INTEGRITY_PASS = 0,
> > @@ -58,6 +59,9 @@ integrity_inode_attrs_stat_changed
> > if (stat->result_mask & STATX_CHANGE_COOKIE)
> > return stat->change_cookie != attrs->version;
> >
> > + if (stat->result_mask & STATX_CTIME)
> > + return timespec64_to_ns(&stat->ctime) != (s64)attrs->version;
> > +
> > return true;
> > }
> >
> > @@ -84,7 +88,7 @@ integrity_inode_attrs_changed(const struct integrity_inode_attributes *attrs,
> > * only for IMA if vfs_getattr_nosec() fails.
> > */
> > if (!file || vfs_getattr_nosec(&file->f_path, &stat,
> > - STATX_CHANGE_COOKIE,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT))
> > return !IS_I_VERSION(inode) ||
> > !inode_eq_iversion(inode, attrs->version);
> > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
> > index c35ea613c9f8d404ba4886e3b736c3bab29d1668..e47d6281febc15a0ac1bd2ea1d28fea4d0cd5c58 100644
> > --- a/security/integrity/ima/ima_api.c
> > +++ b/security/integrity/ima/ima_api.c
> > @@ -272,10 +272,15 @@ int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
> > * to an initial measurement/appraisal/audit, but was modified to
> > * assume the file changed.
> > */
> > - result = vfs_getattr_nosec(&file->f_path, &stat, STATX_CHANGE_COOKIE,
> > + result = vfs_getattr_nosec(&file->f_path, &stat,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT);
> > - if (!result && (stat.result_mask & STATX_CHANGE_COOKIE))
> > - i_version = stat.change_cookie;
> > + if (!result) {
> > + if (stat.result_mask & STATX_CHANGE_COOKIE)
> > + i_version = stat.change_cookie;
> > + else if (stat.result_mask & STATX_CTIME)
> > + i_version = timespec64_to_ns(&stat.ctime);
> > + }
> > hash.hdr.algo = algo;
> > hash.hdr.length = hash_digest_size[algo];
> >
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index 8cb17c9d446caaa5a98f5ec8f027c17ba7babca8..776db158b0bd8a0d053729ac0cc15af8b6020a98 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -199,7 +199,7 @@ static void ima_check_last_writer(struct ima_iint_cache *iint,
> > &iint->atomic_flags);
> > if ((iint->flags & IMA_NEW_FILE) ||
> > vfs_getattr_nosec(&file->f_path, &stat,
> > - STATX_CHANGE_COOKIE,
> > + STATX_CHANGE_COOKIE | STATX_CTIME,
> > AT_STATX_SYNC_AS_STAT) ||
> > integrity_inode_attrs_stat_changed(&iint->real_inode,
> > &stat)) {
> >
>
^ permalink raw reply
* Re: [PATCH v5 3/3] ima: Use kstat.ctime as a fallback change detection for stacked fs
From: Mimi Zohar @ 2026-02-05 4:26 UTC (permalink / raw)
To: Frederick Lawler, Roberto Sassu
Cc: Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Paul Moore,
James Morris, Serge E. Hallyn, Darrick J. Wong, Christian Brauner,
Josef Bacik, Jeff Layton, linux-kernel, linux-integrity,
linux-security-module, kernel-team
In-Reply-To: <aYO4fj0Uw0aUWXOX@CMGLRV3>
On Wed, 2026-02-04 at 15:22 -0600, Frederick Lawler wrote:
> That said, I think Mimi pointed out in an email [2] where multi-grain
> file systems are impacted regardless of stacked fs or not due to the last
> writer check.
>
> I don't recall coming across that in my tests, but perhaps I did that
> specific test wrong? To be sure, I created the C program, and on the VM,
> created a XFS disk, mounted it on loopback, ran the rdwr program on
> "somefile" multiple times, and only got 1 audit log for it, until I
> mutated it with touch, and only got 2 hits: original + after mutation
> after running the program multiple times.
>
> I'm not sure what's going on there, so I'll look into that a bit more,
> but so far the impact is stacked file systems & multigrain ctime AFAIK.
Make sure you're testing without your patch set applied or at least the last
patch.
Mimi
^ permalink raw reply
* Re: [PATCH v3 2/3] ima: trim N IMA event log records
From: steven chen @ 2026-02-05 23:53 UTC (permalink / raw)
To: Roberto Sassu, linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve,
gregorylumen, nramas, sushring, linux-doc, steven chen
In-Reply-To: <74ae5c5c4cb6644537b2643d02b649a2064b718d.camel@huaweicloud.com>
On 1/7/2026 2:06 AM, Roberto Sassu wrote:
> On Mon, 2026-01-05 at 18:07 -0800, steven chen wrote:
>> Trim N entries of the IMA event logs. Clean the hash table if
>> ima_flush_htable is set.
>>
>> Provide a userspace interface ima_trim_log that can be used to input
>> number N to let kernel to trim N entries of IMA event logs. When read
>> this interface, it returns number of entries trimmed last time.
>>
>> Signed-off-by: steven chen <chenste@linux.microsoft.com>
>> ---
>> .../admin-guide/kernel-parameters.txt | 4 +
>> security/integrity/ima/ima.h | 2 +
>> security/integrity/ima/ima_fs.c | 164 +++++++++++++++++-
>> security/integrity/ima/ima_queue.c | 85 +++++++++
>> 4 files changed, 251 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index e92c0056e4e0..cd1a1d0bf0e2 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -2197,6 +2197,10 @@
>> Use the canonical format for the binary runtime
>> measurements, instead of host native format.
>>
>> + ima_flush_htable [IMA]
>> + Flush the measurement list hash table when trim all
>> + or a part of it for deletion.
>> +
>> ima_hash= [IMA]
>> Format: { md5 | sha1 | rmd160 | sha256 | sha384
>> | sha512 | ... }
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index e3d71d8d56e3..2102c523dca0 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -246,8 +246,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
>>
>> #ifdef CONFIG_IMA_KEXEC
>> void ima_measure_kexec_event(const char *event_name);
>> +long ima_delete_event_log(long req_val);
>> #else
>> static inline void ima_measure_kexec_event(const char *event_name) {}
>> +static inline long ima_delete_event_log(long req_val) { return 0; }
>> #endif
>>
>> /*
>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>> index 87045b09f120..67ff0cfc3d3f 100644
>> --- a/security/integrity/ima/ima_fs.c
>> +++ b/security/integrity/ima/ima_fs.c
>> @@ -21,6 +21,9 @@
>> #include <linux/rcupdate.h>
>> #include <linux/parser.h>
>> #include <linux/vmalloc.h>
>> +#include <linux/ktime.h>
>> +#include <linux/timekeeping.h>
>> +#include <linux/ima.h>
>>
>> #include "ima.h"
>>
>> @@ -38,6 +41,17 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
>>
>> static int valid_policy = 1;
>>
>> +#define IMA_LOG_TRIM_REQ_LENGTH 11
>> +#define IMA_LOG_TRIM_EVENT_LEN 256
> Shouldn't this belong to the next patch?
Will update this. Thanks
>> +
>> +static long trimcount;
>> +/* mutex protects atomicity of trimming measurement list
>> + * and also protects atomicity the measurement list read
>> + * write operation.
>> + */
>> +static DEFINE_MUTEX(ima_measure_lock);
>> +static long ima_measure_users;
>> +
>> static ssize_t ima_show_htable_value(char __user *buf, size_t count,
>> loff_t *ppos, atomic_long_t *val)
>> {
>> @@ -202,16 +216,77 @@ static const struct seq_operations ima_measurments_seqops = {
>> .show = ima_measurements_show
>> };
>>
>> +/*
>> + * _ima_measurements_open - open the IMA measurements file
>> + * @inode: inode of the file being opened
>> + * @file: file being opened
>> + * @seq_ops: sequence operations for the file
>> + *
>> + * Returns 0 on success, or negative error code.
>> + * Implements mutual exclusion between readers and writer
>> + * of the measurements file. Multiple readers are allowed,
>> + * but writer get exclusive access only no other readers/writers.
>> + * Readers is not allowed when there is a writer.
>> + */
>> +static int _ima_measurements_open(struct inode *inode, struct file *file,
>> + const struct seq_operations *seq_ops)
>> +{
>> + bool write = !!(file->f_mode & FMODE_WRITE);
>> + int ret;
>> +
>> + if (write && !capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>> +
>> + mutex_lock(&ima_measure_lock);
>> + if ((write && ima_measure_users != 0) ||
>> + (!write && ima_measure_users < 0)) {
>> + mutex_unlock(&ima_measure_lock);
>> + return -EBUSY;
>> + }
>> +
>> + ret = seq_open(file, seq_ops);
>> + if (ret < 0) {
>> + mutex_unlock(&ima_measure_lock);
>> + return ret;
>> + }
>> +
>> + if (write)
>> + ima_measure_users--;
>> + else
>> + ima_measure_users++;
>> +
>> + mutex_unlock(&ima_measure_lock);
>> + return ret;
>> +}
>> +
>> static int ima_measurements_open(struct inode *inode, struct file *file)
>> {
>> - return seq_open(file, &ima_measurments_seqops);
>> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
>> +}
>> +
>> +static int ima_measurements_release(struct inode *inode, struct file *file)
>> +{
>> + bool write = !!(file->f_mode & FMODE_WRITE);
>> + int ret;
>> +
>> + mutex_lock(&ima_measure_lock);
>> + ret = seq_release(inode, file);
>> + if (!ret) {
>> + if (write)
>> + ima_measure_users++;
>> + else
>> + ima_measure_users--;
>> + }
>> +
>> + mutex_unlock(&ima_measure_lock);
>> + return ret;
>> }
>>
>> static const struct file_operations ima_measurements_ops = {
>> .open = ima_measurements_open,
>> .read = seq_read,
>> .llseek = seq_lseek,
>> - .release = seq_release,
>> + .release = ima_measurements_release,
>> };
>>
>> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
>> @@ -279,14 +354,83 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
>>
>> static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
>> {
>> - return seq_open(file, &ima_ascii_measurements_seqops);
>> + return _ima_measurements_open(inode, file, &ima_ascii_measurements_seqops);
>> }
>>
>> static const struct file_operations ima_ascii_measurements_ops = {
>> .open = ima_ascii_measurements_open,
>> .read = seq_read,
>> .llseek = seq_lseek,
>> - .release = seq_release,
>> + .release = ima_measurements_release,
>> +};
>> +
>> +static int ima_log_trim_open(struct inode *inode, struct file *file)
>> +{
>> + bool write = !!(file->f_mode & FMODE_WRITE);
>> +
>> + if (!write && capable(CAP_SYS_ADMIN))
>> + return 0;
>> + else if (!capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>> +
>> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
>> +}
>> +
>> +static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
>> +{
>> + char tmpbuf[IMA_LOG_TRIM_REQ_LENGTH]; /* greater than largest 'long' string value */
>> + ssize_t len;
>> +
>> + len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
>> + return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
>> +}
>> +
>> +static ssize_t ima_log_trim_write(struct file *file,
>> + const char __user *buf, size_t datalen, loff_t *ppos)
>> +{
>> + long count, n, ret;
>> +
>> + if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_LENGTH || datalen < 2) {
>> + ret = -EINVAL;
>> + goto out;
>> + }
>> +
>> + n = (int)datalen;
>> +
>> + ret = kstrtol_from_user(buf, n, 10, &count);
>> + if (ret < 0)
>> + goto out;
>> +
>> + ret = ima_delete_event_log(count);
>> +
>> + if (ret < 0)
>> + goto out;
>> +
>> + trimcount = ret;
>> +
>> + ret = datalen;
>> +out:
>> + return ret;
>> +}
>> +
>> +static int ima_log_trim_release(struct inode *inode, struct file *file)
>> +{
>> + bool write = !!(file->f_mode & FMODE_WRITE);
>> +
>> + if (!write && capable(CAP_SYS_ADMIN))
>> + return 0;
>> + else if (!capable(CAP_SYS_ADMIN))
>> + return -EPERM;
>> +
>> + return ima_measurements_release(inode, file);
>> +}
>> +
>> +static const struct file_operations ima_log_trim_ops = {
>> + .open = ima_log_trim_open,
>> + .read = ima_log_trim_read,
>> + .write = ima_log_trim_write,
>> + .llseek = generic_file_llseek,
>> + .release = ima_log_trim_release
>> };
>>
>> static ssize_t ima_read_policy(char *path)
>> @@ -528,6 +672,18 @@ int __init ima_fs_init(void)
>> goto out;
>> }
>>
>> + if (IS_ENABLED(CONFIG_IMA_LOG_TRIMMING)) {
>> + dentry = securityfs_create_file("ima_trim_log",
>> + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
>> + ima_dir, NULL, &ima_log_trim_ops);
>> + if (IS_ERR(dentry)) {
>> + ret = PTR_ERR(dentry);
>> + goto out;
>> + }
>> + }
>> +
>> + trimcount = 0;
>> +
>> dentry = securityfs_create_file("runtime_measurements_count",
>> S_IRUSR | S_IRGRP, ima_dir, NULL,
>> &ima_measurements_count_ops);
>> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
>> index 590637e81ad1..33bb5414b8cc 100644
>> --- a/security/integrity/ima/ima_queue.c
>> +++ b/security/integrity/ima/ima_queue.c
>> @@ -22,6 +22,14 @@
>>
>> #define AUDIT_CAUSE_LEN_MAX 32
>>
>> +bool ima_flush_htable;
>> +static int __init ima_flush_htable_setup(char *str)
>> +{
>> + ima_flush_htable = true;
>> + return 1;
>> +}
>> +__setup("ima_flush_htable", ima_flush_htable_setup);
>> +
>> /* pre-allocated array of tpm_digest structures to extend a PCR */
>> static struct tpm_digest *digests;
>>
>> @@ -220,6 +228,83 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>> return result;
>> }
>>
>> +/**
>> + * ima_delete_event_log - delete IMA event entry
>> + * @num_records: number of records to delete
>> + *
>> + * delete num_records entries off the measurement list.
>> + * Returns the number of entries deleted, or negative error code.
> This is not according to the format stated in the documentation.
Will update this. Thanks
>> + */
>> +long ima_delete_event_log(long num_records)
>> +{
>> + long len, cur = num_records, tmp_len = 0;
>> + struct ima_queue_entry *qe, *qe_tmp;
>> + LIST_HEAD(ima_measurements_staged);
>> + struct list_head *list_ptr;
>> +
>> + if (num_records <= 0)
>> + return num_records;
>> +
>> + if (!IS_ENABLED(CONFIG_IMA_LOG_TRIMMING))
>> + return -EOPNOTSUPP;
>> +
>> + mutex_lock(&ima_extend_list_mutex);
>> + len = atomic_long_read(&ima_htable.len);
>> +
>> + if (num_records > len) {
>> + mutex_unlock(&ima_extend_list_mutex);
>> + return -ENOENT;
>> + }
>> +
>> + list_ptr = &ima_measurements;
>> +
>> + if (cur == len) {
>> + list_replace(&ima_measurements, &ima_measurements_staged);
>> + INIT_LIST_HEAD(&ima_measurements);
>> + atomic_long_set(&ima_htable.len, 0);
>> + list_ptr = &ima_measurements_staged;
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size = 0;
> Like in my patch, we should have kept the original value of
> binary_runtime_size, to avoid breaking the kexec critical data records.
I think this value is updated when trimming finish.
>> + }
>> +
>> + list_for_each_entry(qe, list_ptr, later) {
>> + if (num_records > 0) {
>> + if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE) && ima_flush_htable)
>> + hlist_del_rcu(&qe->hnext);
>> +
>> + --num_records;
>> + if (num_records == 0)
>> + qe_tmp = qe;
>> + continue;
>> + }
>> + if (len != cur && IS_ENABLED(CONFIG_IMA_KEXEC))
>> + tmp_len += get_binary_runtime_size(qe->entry);
>> + else
>> + break;
>> + }
>> +
>> + if (len != cur) {
>> + __list_cut_position(&ima_measurements_staged, &ima_measurements,
>> + &qe_tmp->later);
>> + atomic_long_sub(cur, &ima_htable.len);
>> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
>> + binary_runtime_size = tmp_len;
>> + }
>> +
>> + mutex_unlock(&ima_extend_list_mutex);
>> +
>> + if (ima_flush_htable)
>> + synchronize_rcu();
>> +
>> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_staged, later) {
>> + ima_free_template_entry(qe->entry);
>> + list_del(&qe->later);
>> + kfree(qe);
> If you don't flush the hash table, you cannot delete the entry.
>
> Roberto
Will update this. Thanks
Steven
>> + }
>> +
>> + return cur;
>> +}
>> +
>> int ima_restore_measurement_entry(struct ima_template_entry *entry)
>> {
>> int result = 0;
^ permalink raw reply
* [PATCH v4 0/1] Trim N entries of IMA event logs
From: steven chen @ 2026-02-05 23:58 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring, linux-doc
The Integrity Measurement Architecture (IMA) maintains a measurement list
—a record of system events used for integrity verification. The IMA event
logs are the entries within this measurement list, each representing a
specific event or measurement that contributes to the system's integrity
assessment.
This update introduces the ability to trim, or remove, N entries from the
current measurement list. Trimming involves deleting N entries from the
list and clearing the corresponding entries from the hash table. This
action atomically truncates the measurement list, ensuring that no new
measurements can be added until the operation is complete. Importantly,
only one writer can initiate this trimming process at a time, maintaining
consistency and preventing race conditions.
A userspace interface, ima_trim_log, has been provided for this purpose.
When this interface is read, it returns the total number T of entries
trimmed since system boot up. This value T need to be preserved across
kexec soft reboots. By writing two number T:N to this interface, userspace
can request the kernel to trim N entries from the IMA event logs.
To maintain a complete record, userspace is responsible for concatenating
and storing the logs before initiating trimming. Userspace can then send
the collected data to remote verifiers for validation. After receiving
confirmation from the remote verifiers, userspace may instruct the kernel
to proceed with trimming the IMA event logs accordingly.
The primary benefit of this solution is the ability to free valuable
kernel memory by delegating the task of reconstructing the full
measurement list from log chunks to userspace. Trust is not required in
userspace for the integrity of the measurement list, as its integrity is
cryptographically protected by the Trusted Platform Module (TPM).
Multiple readers are allowed to access the ima_trim_log interface
concurrently, while only one writer can trigger log trimming at any time.
During trimming, readers do not see the list and cannot access it while
deletion is in progress, ensuring atomicity.
Introduce the new kernel option ima_flush_htable to decide whether or not
the digests of measurement entries are flushed from the hash table (from
reference [2]).
The ima_measure_users counter (protected by the ima_measure_lock mutex) has
been introduced to protect access to the measurement list part. The open
method of all the measurement interfaces has been extended to allow only
one writer at a time or, in alternative, multiple readers. The write
permission is used to stage/delete the measurements, the read permission
to read them. Write requires also the CAP_SYS_ADMIN capability (from
reference [2]). This ima_measure_users needs to be preserved across kexec
soft reboots
The total trimmed number T and the ima_measure_users both need to be
preserved across kexec soft reboot and new patch will be added for this
purpose in next version.
New IMA log trim event is added when trimming finish.
The time required for trimming is minimal, and IMA event logs are briefly
on hold during this process, preventing read or add operations. This short
interruption has no impact on the overall functionality of IMA.
A new critical data record "ima_log_trim" is added in this solution. This
record logs the trim event with number of entries deleted total T since
system start and time when this happened. User space can get the total
number T of entries trimmed by checking "ima_log_trim" event in the
measurement list.
The following are how user space to use the measurement list and
ima_log_trim interface
1. get the total numer trimmed T through "ima_log_trim" interface
2. get the PCR quote
3. read the measurement list file, close the file, send for verification
4. wait for response from verifier, until get the good response from
verifier with number N that matched the PCR quote got in step 2
5. get the number N from the above message
6. write the T:N to the ima_log_trim interface when no conflict
when kernel get log trim request T:N
Get the T, compare with the total trimmed number
if equal, then do trim N and change T to T+N
else return error
Using above way to trim the log, the time for user space to hold the list
will be trimming T:N operation itself at the step 6. User space agent
race condition is solved too in this way.
References:
-----------
[1] [PATCH 0/1] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/20251202232857.8211-1-chenste@linux.microsoft.com/T/#t
[2] [RFC][PATCH] ima: Add support for staging measurements for deletion
https://lore.kernel.org/linux-integrity/207fd6d7-53c-57bb-36d8-13a0902052d1@linux.microsoft.com/T/#t
[3] [PATCH v2 0/1] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/20251210235314.3341-1-chenste@linux.microsoft.com/T/#t
[4] [PATCH v3 0/3] Trim N entries of IMA event logs
https://lore.kernel.org/linux-integrity/20260106020713.3994-1-chenste@linux.microsoft.com/T/#t
Change Log v4:
- Incorporated feedback from Roberto on v3 series.
- Update "ima_log_trim" interface definition
When read this interface, return total number of records trimmed T
need to write T:N to this interface to trim N records
- Update user space use case on how to trim IMA event logs
- Updated patch descriptions as necessary.
Change Log v3:
- Incorporated feedback from Mimi on v2 series.
- split patch into multiple patches
- lock time performance improvement
- Updated patch descriptions as necessary.
Change Log v2:
- Incorporated feedback from the Roberto on v1 series.
- Adapted code from Roberto's RFC [Reference 2]
- Add IMA log trim event log to record trim event
- Updated patch descriptions as necessary.
steven chen (3):
ima: make ima event log trimming configurable
ima: trim N IMA event log records
ima: add new critical data record to measure log trim
.../admin-guide/kernel-parameters.txt | 4 +
security/integrity/ima/Kconfig | 12 +
security/integrity/ima/ima.h | 2 +
security/integrity/ima/ima_fs.c | 214 +++++++++++++++++-
security/integrity/ima/ima_queue.c | 100 ++++++++
5 files changed, 328 insertions(+), 4 deletions(-)
--
2.43.0
^ permalink raw reply
* [PATCH v4 1/3] ima: make ima event log trimming configurable
From: steven chen @ 2026-02-05 23:58 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring, linux-doc
In-Reply-To: <20260205235849.7086-1-chenste@linux.microsoft.com>
Make ima event log trimming function configurable.
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
security/integrity/ima/Kconfig | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 976e75f9b9ba..322964ae4772 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -332,4 +332,16 @@ config IMA_KEXEC_EXTRA_MEMORY_KB
If set to the default value of 0, an extra half page of memory for those
additional measurements will be allocated.
+config IMA_LOG_TRIMMING
+ bool "IMA Event Log Trimming"
+ default n
+ help
+ Say Y here if you want support for IMA Event Log Trimming.
+ This creates the file /sys/kernel/security/integrity/ima/ima_trim_log.
+ Userspace
+ - writes to this file to trigger IMA event log trimming
+ - reads this file to get number of entried trimming last time
+
+ If unsure, say N.
+
endif
--
2.43.0
^ permalink raw reply related
* [PATCH v4 2/3] ima: trim N IMA event log records
From: steven chen @ 2026-02-05 23:58 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring, linux-doc
In-Reply-To: <20260205235849.7086-1-chenste@linux.microsoft.com>
Trim N entries of the IMA event logs. Clean the hash table if
ima_flush_htable is set.
Provide a userspace interface ima_trim_log:
When read this interface, it returns total number T of entries trimmed
since system boot up.
When write to this interface need to provide two numbers T:N to let
kernel to trim N entries of IMA event logs.
when kernel get log trim request T:N
- Get the T, compare with the total trimmed number
- if equal, then do trim N and change T to T+N
- else return error
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
.../admin-guide/kernel-parameters.txt | 4 +
security/integrity/ima/ima.h | 2 +
security/integrity/ima/ima_fs.c | 195 +++++++++++++++++-
security/integrity/ima/ima_queue.c | 100 +++++++++
4 files changed, 297 insertions(+), 4 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index e92c0056e4e0..cd1a1d0bf0e2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2197,6 +2197,10 @@
Use the canonical format for the binary runtime
measurements, instead of host native format.
+ ima_flush_htable [IMA]
+ Flush the measurement list hash table when trim all
+ or a part of it for deletion.
+
ima_hash= [IMA]
Format: { md5 | sha1 | rmd160 | sha256 | sha384
| sha512 | ... }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e3d71d8d56e3..2102c523dca0 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -246,8 +246,10 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
#ifdef CONFIG_IMA_KEXEC
void ima_measure_kexec_event(const char *event_name);
+long ima_delete_event_log(long req_val);
#else
static inline void ima_measure_kexec_event(const char *event_name) {}
+static inline long ima_delete_event_log(long req_val) { return 0; }
#endif
/*
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 87045b09f120..7f805ab62f6c 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -21,6 +21,9 @@
#include <linux/rcupdate.h>
#include <linux/parser.h>
#include <linux/vmalloc.h>
+#include <linux/ktime.h>
+#include <linux/timekeeping.h>
+#include <linux/ima.h>
#include "ima.h"
@@ -38,6 +41,17 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
static int valid_policy = 1;
+#define IMA_LOG_TRIM_REQ_NUM_LENGTH 15
+#define IMA_LOG_TRIM_REQ_TOTAL_LENGTH 32
+
+static long trimcount;
+/* mutex protects atomicity of trimming measurement list
+ * and also protects atomicity the measurement list read
+ * write operation.
+ */
+static DEFINE_MUTEX(ima_measure_lock);
+static long ima_measure_users;
+
static ssize_t ima_show_htable_value(char __user *buf, size_t count,
loff_t *ppos, atomic_long_t *val)
{
@@ -202,16 +216,77 @@ static const struct seq_operations ima_measurments_seqops = {
.show = ima_measurements_show
};
+/*
+ * _ima_measurements_open - open the IMA measurements file
+ * @inode: inode of the file being opened
+ * @file: file being opened
+ * @seq_ops: sequence operations for the file
+ *
+ * Returns 0 on success, or negative error code.
+ * Implements mutual exclusion between readers and writer
+ * of the measurements file. Multiple readers are allowed,
+ * but writer get exclusive access only no other readers/writers.
+ * Readers is not allowed when there is a writer.
+ */
+static int _ima_measurements_open(struct inode *inode, struct file *file,
+ const struct seq_operations *seq_ops)
+{
+ bool write = !!(file->f_mode & FMODE_WRITE);
+ int ret;
+
+ if (write && !capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ mutex_lock(&ima_measure_lock);
+ if ((write && ima_measure_users != 0) ||
+ (!write && ima_measure_users < 0)) {
+ mutex_unlock(&ima_measure_lock);
+ return -EBUSY;
+ }
+
+ ret = seq_open(file, seq_ops);
+ if (ret < 0) {
+ mutex_unlock(&ima_measure_lock);
+ return ret;
+ }
+
+ if (write)
+ ima_measure_users--;
+ else
+ ima_measure_users++;
+
+ mutex_unlock(&ima_measure_lock);
+ return ret;
+}
+
static int ima_measurements_open(struct inode *inode, struct file *file)
{
- return seq_open(file, &ima_measurments_seqops);
+ return _ima_measurements_open(inode, file, &ima_measurments_seqops);
+}
+
+static int ima_measurements_release(struct inode *inode, struct file *file)
+{
+ bool write = !!(file->f_mode & FMODE_WRITE);
+ int ret;
+
+ mutex_lock(&ima_measure_lock);
+ ret = seq_release(inode, file);
+ if (!ret) {
+ if (!write)
+ ima_measure_users--;
+ else
+ ima_measure_users++;
+ }
+
+ mutex_unlock(&ima_measure_lock);
+ return ret;
}
static const struct file_operations ima_measurements_ops = {
.open = ima_measurements_open,
.read = seq_read,
.llseek = seq_lseek,
- .release = seq_release,
+ .release = ima_measurements_release,
};
void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
@@ -279,14 +354,114 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
{
- return seq_open(file, &ima_ascii_measurements_seqops);
+ return _ima_measurements_open(inode, file, &ima_ascii_measurements_seqops);
}
static const struct file_operations ima_ascii_measurements_ops = {
.open = ima_ascii_measurements_open,
.read = seq_read,
.llseek = seq_lseek,
- .release = seq_release,
+ .release = ima_measurements_release,
+};
+
+static int ima_log_trim_open(struct inode *inode, struct file *file)
+{
+ bool write = !!(file->f_mode & FMODE_WRITE);
+
+ if (!write && capable(CAP_SYS_ADMIN))
+ return 0;
+ else if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ return _ima_measurements_open(inode, file, &ima_measurments_seqops);
+}
+
+static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
+{
+ char tmpbuf[IMA_LOG_TRIM_REQ_NUM_LENGTH];
+ ssize_t len;
+
+ len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
+ return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
+}
+
+static ssize_t ima_log_trim_write(struct file *file,
+ const char __user *buf, size_t datalen, loff_t *ppos)
+{
+ char tmpbuf[IMA_LOG_TRIM_REQ_TOTAL_LENGTH];
+ char *p = tmpbuf;
+ long count, ret, val = 0, max = LONG_MAX;
+
+ if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_TOTAL_LENGTH || datalen < 2) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ if (copy_from_user(tmpbuf, buf, datalen) != 0) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ p = tmpbuf;
+
+ while (*p && *p != ':') {
+ if (!isdigit((unsigned char)*p))
+ return -EINVAL;
+
+ /* digit value */
+ int d = *p - '0';
+
+ /* overflow check: val * 10 + d > max -> (val > (max - d) / 10) */
+ if (val > (max - d) / 10)
+ return -ERANGE;
+
+ val = val * 10 + d;
+ p++;
+ }
+
+ if (*p != ':')
+ return -EINVAL;
+
+ /* verify trim count matches */
+ if (val != trimcount)
+ return -EINVAL;
+
+ p++; /* skip ':' */
+ ret = kstrtoul(p, 0, &count);
+
+ if (ret < 0)
+ goto out;
+
+ ret = ima_delete_event_log(count);
+
+ if (ret < 0)
+ goto out;
+
+ trimcount += ret;
+
+ ret = datalen;
+out:
+ return ret;
+}
+
+static int ima_log_trim_release(struct inode *inode, struct file *file)
+{
+ bool write = !!(file->f_mode & FMODE_WRITE);
+
+ if (!write && capable(CAP_SYS_ADMIN))
+ return 0;
+ else if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ return ima_measurements_release(inode, file);
+}
+
+static const struct file_operations ima_log_trim_ops = {
+ .open = ima_log_trim_open,
+ .read = ima_log_trim_read,
+ .write = ima_log_trim_write,
+ .llseek = generic_file_llseek,
+ .release = ima_log_trim_release
};
static ssize_t ima_read_policy(char *path)
@@ -528,6 +703,18 @@ int __init ima_fs_init(void)
goto out;
}
+ if (IS_ENABLED(CONFIG_IMA_LOG_TRIMMING)) {
+ dentry = securityfs_create_file("ima_trim_log",
+ S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
+ ima_dir, NULL, &ima_log_trim_ops);
+ if (IS_ERR(dentry)) {
+ ret = PTR_ERR(dentry);
+ goto out;
+ }
+ }
+
+ trimcount = 0;
+
dentry = securityfs_create_file("runtime_measurements_count",
S_IRUSR | S_IRGRP, ima_dir, NULL,
&ima_measurements_count_ops);
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 590637e81ad1..5ef722d0fa24 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -22,6 +22,14 @@
#define AUDIT_CAUSE_LEN_MAX 32
+bool ima_flush_htable;
+static int __init ima_flush_htable_setup(char *str)
+{
+ ima_flush_htable = true;
+ return 1;
+}
+__setup("ima_flush_htable", ima_flush_htable_setup);
+
/* pre-allocated array of tpm_digest structures to extend a PCR */
static struct tpm_digest *digests;
@@ -220,6 +228,98 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
return result;
}
+/**
+ * ima_delete_event_log - delete IMA event entry
+ * @num_records: number of records to delete
+ *
+ * delete num_records entries off the measurement list.
+ * Returns num_records, or negative error code.
+ */
+long ima_delete_event_log(long num_records)
+{
+ long len, cur = num_records, tmp_len = 0;
+ struct ima_queue_entry *qe, *qe_tmp;
+ LIST_HEAD(ima_measurements_to_delete);
+ struct list_head *list_ptr;
+
+ if (!IS_ENABLED(CONFIG_IMA_LOG_TRIMMING))
+ return -EOPNOTSUPP;
+
+ if (num_records <= 0)
+ return num_records;
+
+ mutex_lock(&ima_extend_list_mutex);
+ len = atomic_long_read(&ima_htable.len);
+
+ if (num_records > len) {
+ mutex_unlock(&ima_extend_list_mutex);
+ return -ENOENT;
+ }
+
+ list_ptr = &ima_measurements;
+
+ if (num_records == len) {
+ list_replace(&ima_measurements, &ima_measurements_to_delete);
+ INIT_LIST_HEAD(&ima_measurements);
+ atomic_long_set(&ima_htable.len, 0);
+ list_ptr = &ima_measurements_to_delete;
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ binary_runtime_size = 0;
+ }
+
+ list_for_each_entry(qe, list_ptr, later) {
+ if (cur > 0) {
+ if (!IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE) && ima_flush_htable)
+ hlist_del_rcu(&qe->hnext);
+
+ --cur;
+ if (cur == 0)
+ qe_tmp = qe;
+ continue;
+ }
+ if (len != num_records && IS_ENABLED(CONFIG_IMA_KEXEC))
+ tmp_len += get_binary_runtime_size(qe->entry);
+ else
+ break;
+ }
+
+ if (len != num_records) {
+ __list_cut_position(&ima_measurements_to_delete, &ima_measurements,
+ &qe_tmp->later);
+ atomic_long_sub(num_records, &ima_htable.len);
+ if (IS_ENABLED(CONFIG_IMA_KEXEC))
+ binary_runtime_size = tmp_len;
+ }
+
+ mutex_unlock(&ima_extend_list_mutex);
+
+ if (ima_flush_htable)
+ synchronize_rcu();
+
+ list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_to_delete, later) {
+ /*
+ * Ok because after list delete qe is only accessed by
+ * ima_lookup_digest_entry().
+ */
+ for (int i = 0; i < qe->entry->template_desc->num_fields; i++) {
+ kfree(qe->entry->template_data[i].data);
+ qe->entry->template_data[i].data = NULL;
+ qe->entry->template_data[i].len = 0;
+ }
+
+ list_del(&qe->later);
+
+ /* No leak if !ima_flush_htable, referenced by ima_htable. */
+ if (ima_flush_htable) {
+ kfree(qe->entry->digests);
+ kfree(qe->entry);
+ kfree(qe);
+ }
+ }
+
+ return num_records;
+}
+
int ima_restore_measurement_entry(struct ima_template_entry *entry)
{
int result = 0;
--
2.43.0
^ permalink raw reply related
* [PATCH v4 3/3] ima: add new critical data record to measure log trim
From: steven chen @ 2026-02-05 23:58 UTC (permalink / raw)
To: linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve, chenste,
gregorylumen, nramas, sushring, linux-doc
In-Reply-To: <20260205235849.7086-1-chenste@linux.microsoft.com>
Add a new critical data record to measure the trimming event when
ima event records are deleted since system boot up.
If all IMA event logs are saved in the userspace, use this log to get total
numbers of records deleted since system boot up at that point.
Signed-off-by: steven chen <chenste@linux.microsoft.com>
---
security/integrity/ima/ima_fs.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 7f805ab62f6c..1d6befa51044 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -43,6 +43,7 @@ static int valid_policy = 1;
#define IMA_LOG_TRIM_REQ_NUM_LENGTH 15
#define IMA_LOG_TRIM_REQ_TOTAL_LENGTH 32
+#define IMA_LOG_TRIM_EVENT_LEN 256
static long trimcount;
/* mutex protects atomicity of trimming measurement list
@@ -364,6 +365,22 @@ static const struct file_operations ima_ascii_measurements_ops = {
.release = ima_measurements_release,
};
+static void ima_measure_trim_event(void)
+{
+ char ima_log_trim_event[IMA_LOG_TRIM_EVENT_LEN];
+ struct timespec64 ts;
+ u64 time_ns;
+ int n;
+
+ ktime_get_real_ts64(&ts);
+ time_ns = (u64)ts.tv_sec * 1000000000ULL + ts.tv_nsec;
+ n = scnprintf(ima_log_trim_event, IMA_LOG_TRIM_EVENT_LEN,
+ "time= %llu; number= %lu;", time_ns, trimcount);
+
+ ima_measure_critical_data("ima_log_trim", "trim ima event logs",
+ ima_log_trim_event, n, false, NULL, 0);
+}
+
static int ima_log_trim_open(struct inode *inode, struct file *file)
{
bool write = !!(file->f_mode & FMODE_WRITE);
@@ -438,6 +455,8 @@ static ssize_t ima_log_trim_write(struct file *file,
goto out;
trimcount += ret;
+ if (ret > 0)
+ ima_measure_trim_event();
ret = datalen;
out:
--
2.43.0
^ permalink raw reply related
* [PATCH v2 v2] evm: check return values of crypto_shash functions
From: Daniel Hodges @ 2026-02-06 2:42 UTC (permalink / raw)
To: Roberto Sassu
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge, linux-integrity, linux-security-module,
linux-kernel, Daniel Hodges
In-Reply-To: <aYNprpzxppKE0Gf2@fb.com>
The crypto_shash_update() and crypto_shash_final() functions can fail
and return error codes, but their return values were not being checked
in several places in security/integrity/evm/evm_crypto.c:
- hmac_add_misc() ignored returns from crypto_shash_update() and
crypto_shash_final()
- evm_calc_hmac_or_hash() ignored returns from crypto_shash_update()
- evm_init_hmac() ignored returns from crypto_shash_update()
If these hash operations fail silently, the resulting HMAC could be
invalid or incomplete, which could weaken the integrity verification
security that EVM provides.
This patch converts hmac_add_misc() from void to int return type and
adds proper error checking and propagation for all crypto_shash_*
function calls. All callers are updated to handle the new return values.
Additionally, error messages are logged when cryptographic operations
fail to provide visibility into the failure rather than silently
returning error codes.
Fixes: 66dbc325afce ("evm: re-release")
Signed-off-by: Daniel Hodges <git@danielhodges.dev>
---
security/integrity/evm/evm_crypto.c | 55 ++++++++++++++++++++++-------
1 file changed, 42 insertions(+), 13 deletions(-)
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index a5e730ffda57..402eb1ca64ce 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -139,7 +139,7 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
* (Additional directory/file metadata needs to be added for more complete
* protection.)
*/
-static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
+static int hmac_add_misc(struct shash_desc *desc, struct inode *inode,
char type, char *digest)
{
struct h_misc {
@@ -149,6 +149,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
gid_t gid;
umode_t mode;
} hmac_misc;
+ int error;
memset(&hmac_misc, 0, sizeof(hmac_misc));
/* Don't include the inode or generation number in portable
@@ -169,14 +170,28 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
hmac_misc.mode = inode->i_mode;
- crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
+ error = crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
+ if (error) {
+ pr_err("crypto_shash_update() failed: %d\n", error);
+ return error;
+ }
if ((evm_hmac_attrs & EVM_ATTR_FSUUID) &&
- type != EVM_XATTR_PORTABLE_DIGSIG)
- crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
- crypto_shash_final(desc, digest);
+ type != EVM_XATTR_PORTABLE_DIGSIG) {
+ error = crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
+ if (error) {
+ pr_err("crypto_shash_update() failed: %d\n", error);
+ return error;
+ }
+ }
+ error = crypto_shash_final(desc, digest);
+ if (error) {
+ pr_err("crypto_shash_final() failed: %d\n", error);
+ return error;
+ }
pr_debug("hmac_misc: (%zu) [%*phN]\n", sizeof(struct h_misc),
(int)sizeof(struct h_misc), &hmac_misc);
+ return 0;
}
/*
@@ -260,9 +275,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
if ((req_xattr_name && req_xattr_value)
&& !strcmp(xattr->name, req_xattr_name)) {
- error = 0;
- crypto_shash_update(desc, (const u8 *)req_xattr_value,
+ error = crypto_shash_update(desc, (const u8 *)req_xattr_value,
req_xattr_value_len);
+ if (error) {
+ pr_err("crypto_shash_update() failed: %d\n", error);
+ goto out;
+ }
if (is_ima)
ima_present = true;
@@ -286,15 +304,20 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
pr_debug("file %s: xattr %s size mismatch (kernel: %d, user: %d)\n",
dentry->d_name.name, xattr->name, size,
user_space_size);
- error = 0;
xattr_size = size;
- crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
+ error = crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
+ if (error) {
+ pr_err("crypto_shash_update() failed: %d\n", error);
+ goto out;
+ }
if (is_ima)
ima_present = true;
dump_security_xattr(xattr->name, xattr_value, xattr_size);
}
- hmac_add_misc(desc, inode, type, data->digest);
+ error = hmac_add_misc(desc, inode, type, data->digest);
+ if (error)
+ goto out;
if (inode != d_backing_inode(dentry) && iint) {
if (IS_I_VERSION(inode))
@@ -401,6 +424,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
{
struct shash_desc *desc;
const struct xattr *xattr;
+ int error;
desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
if (IS_ERR(desc)) {
@@ -412,12 +436,17 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
if (!evm_protected_xattr(xattr->name))
continue;
- crypto_shash_update(desc, xattr->value, xattr->value_len);
+ error = crypto_shash_update(desc, xattr->value, xattr->value_len);
+ if (error) {
+ pr_err("crypto_shash_update() failed: %d\n", error);
+ goto out;
+ }
}
- hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
+ error = hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
+out:
kfree(desc);
- return 0;
+ return error;
}
/*
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v4 05/17] module: Switch load_info::len to size_t
From: David Howells @ 2026-02-06 8:18 UTC (permalink / raw)
To: =?utf-8?q?Thomas_Wei=C3=9Fschuh?=
Cc: dhowells, Nathan Chancellor, Arnd Bergmann, Luis Chamberlain,
Petr Pavlu, Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Nicolas Schier,
Daniel Gomez, Aaron Tomlin, Christophe Leroy (CS GROUP),
Nicolas Schier, Nicolas Bouchinet, Xiu Jianfeng,
Fabian Grünbichler, Arnout Engelen, Mattia Rizzolo, kpcyrd,
Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260113-module-hashes-v4-5-0b932db9b56b@weissschuh.net>
Thomas Weißschuh <linux@weissschuh.net> wrote:
> As both 'size_t' and 'unsigned int' are always the same size, this
> should be risk-free.
Did you mean 'unsigned long'?
David
^ permalink raw reply
* Re: [PATCH v4 02/17] powerpc/ima: Drop unnecessary check for CONFIG_MODULE_SIG
From: Nicolas Schier @ 2026-02-06 8:25 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Daniel Gomez,
Aaron Tomlin, Christophe Leroy (CS GROUP), Nicolas Bouchinet,
Xiu Jianfeng, Fabian Grünbichler, Arnout Engelen,
Mattia Rizzolo, kpcyrd, Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260113-module-hashes-v4-2-0b932db9b56b@weissschuh.net>
On Tue, Jan 13, 2026 at 01:28:46PM +0100, Thomas Weißschuh wrote:
> When CONFIG_MODULE_SIG is disabled set_module_sig_enforced() is defined
> as an empty stub, so the check is unnecessary.
> The specific configuration option for set_module_sig_enforced() is
> about to change and removing the check avoids some later churn.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> arch/powerpc/kernel/ima_arch.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
Reviewed-by: Nicolas Schier <nsc@kernel.org>
--
Nicolas
^ permalink raw reply
* Re: [PATCH v4 03/17] ima: efi: Drop unnecessary check for CONFIG_MODULE_SIG/CONFIG_KEXEC_SIG
From: Nicolas Schier @ 2026-02-06 8:25 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Nathan Chancellor, Arnd Bergmann, Luis Chamberlain, Petr Pavlu,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Daniel Gomez,
Aaron Tomlin, Christophe Leroy (CS GROUP), Nicolas Bouchinet,
Xiu Jianfeng, Fabian Grünbichler, Arnout Engelen,
Mattia Rizzolo, kpcyrd, Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <20260113-module-hashes-v4-3-0b932db9b56b@weissschuh.net>
On Tue, Jan 13, 2026 at 01:28:47PM +0100, Thomas Weißschuh wrote:
> When configuration settings are disabled the guarded functions are
> defined as empty stubs, so the check is unnecessary.
> The specific configuration option for set_module_sig_enforced() is
> about to change and removing the checks avoids some later churn.
>
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> security/integrity/ima/ima_efi.c | 6 ++----
> 1 file changed, 2 insertions(+), 4 deletions(-)
>
Reviewed-by: Nicolas Schier <nsc@kernel.org>
--
Nicolas
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox