Re: [Qemu-devel] [libseccomp-discuss] [RFC] [PATCHv2 0/2] Sandboxing Qemu guests with Libseccomp
On Wed, Jun 13, 2012 at 1:31 PM, Paul Moore wrote: > On Wednesday, June 13, 2012 04:20:20 PM Eduardo Otubo wrote: >> Hello all, >> >> This is the second effort to sandbox Qemu guests using Libseccomp[0]. > > ... > >> [0] - http://sourceforge.net/projects/libseccomp/ [1] - >> http://git.kernel.org/?p=linux/kernel/git/next/linux-next.git;a=commit;h=e2c >> fabdfd075648216f99c2c03821cf3f47c1727 > > It is worth pointing out that you no longer need to fetch libseccomp from the > git repository, we did our first release (v 0.1.0) last Friday, June 8th: > > * https://sourceforge.net/projects/libseccomp/files > > Packages are available for Debian/Ubuntu and Fedora packaging is currently in > progress. Gentoo has en ebuild as well. If you hit any snags with the packaging there or in Debian and Ubuntu, let me know. :) -Kees -- Kees Cook Chrome OS Security
Re: [Qemu-devel] [RFC PATCH 00/11] Adding FreeBSD's Capsicum security framework (part 1)
On Mon, Jul 7, 2014 at 3:33 PM, Alexei Starovoitov wrote: > On Mon, Jul 7, 2014 at 5:20 AM, Paolo Bonzini wrote: >> Il 07/07/2014 12:29, David Drysdale ha scritto: >> >>>> I think that's more easily done by opening the file as O_RDONLY/O_WRONLY >>>> /O_RDWR. You could do it by running the file descriptor's seccomp-bpf >>>> program once per iocb with synthesized syscall numbers and argument >>>> vectors. >>> >>> >>> Right, but generating the equivalent seccomp input environment for an >>> equivalent single-fd syscall is going to be subtle and complex (which >>> are worrying words to mention in a security context). And how many >>> other syscalls are going to need similar special-case processing? >>> (poll? select? send[m]msg? ...) >> >> >> Yeah, the difficult part is getting the right balance between: >> >> 1) limitations due to seccomp's impossibility to chase pointers (which is >> not something that can be lifted, as it's required for correctness) > > btw once seccomp moves to eBPF it will be able to 'chase pointers', > since pointer walking will be possible via bpf_load_pointer() function call, > which is a wrapper of: > probe_kernel_read(&ptr, unsafe_ptr, sizeof(void *)); > return ptr; > Not sure whether it helps this case or not. Just fyi. It won't immediately help, since threads can race pointer target contents (i.e. seccomp sees one thing, and then the syscall see another thing). Having an immutable memory area could help with this (i.e. some kind of "locked" memory range that holds all the "approved" argument strings, at which point seccomp could then trust the chased pointers that land in this range.) Obviously eBPF is a prerequisite to this, but it isn't the full solution, as far as I understand it. -Kees -- Kees Cook Chrome OS Security
[Qemu-devel] [Bug 697197] Re: Empty password allows access to VNC in libvirt
Thanks for preparing the debdiffs! It looks like karmic is vulnerable
too, so we'll need that as well. I'll update the debdiffs to use proper
DEP-3 and fix up the formatting of the changelogs a bit ("CVE-" vs "CVE:
"), and get these building.
** Also affects: libvirt (Ubuntu Karmic)
Importance: Undecided
Status: New
** Also affects: qemu-kvm (Ubuntu Karmic)
Importance: Undecided
Status: New
** Changed in: libvirt (Ubuntu Karmic)
Status: New => Invalid
** Changed in: qemu-kvm (Ubuntu Karmic)
Status: New => In Progress
--
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/697197
Title:
Empty password allows access to VNC in libvirt
Status in libvirt virtualization API:
Unknown
Status in QEMU:
Confirmed
Status in qemu-kvm:
Unknown
Status in “libvirt” package in Ubuntu:
Invalid
Status in “qemu-kvm” package in Ubuntu:
Fix Released
Status in “libvirt” source package in Lucid:
Invalid
Status in “qemu-kvm” source package in Lucid:
In Progress
Status in “libvirt” source package in Maverick:
Invalid
Status in “qemu-kvm” source package in Maverick:
In Progress
Status in “libvirt” source package in Natty:
Invalid
Status in “qemu-kvm” source package in Natty:
Fix Released
Status in “libvirt” source package in Karmic:
Invalid
Status in “qemu-kvm” source package in Karmic:
In Progress
Bug description:
The help in the /etc/libvirt/qemu.conf states
"To allow access without passwords, leave this commented out. An empty
string will still enable passwords, but be rejected by QEMU
effectively preventing any use of VNC."
yet setting:
vnc_password=""
allows access to the vnc console without any password prompt just as
if it is hashed out completely.
ProblemType: Bug
DistroRelease: Ubuntu 10.10
Package: libvirt-bin 0.8.3-1ubuntu14
ProcVersionSignature: Ubuntu 2.6.35-24.42-server 2.6.35.8
Uname: Linux 2.6.35-24-server x86_64
Architecture: amd64
Date: Tue Jan 4 12:18:35 2011
InstallationMedia: Ubuntu-Server 10.04.1 LTS "Lucid Lynx" - Release amd64
(20100816.2)
ProcEnviron:
LANG=en_GB.UTF-8
SHELL=/bin/bash
SourcePackage: libvirt
[Qemu-devel] [Bug 697197] Re: Empty password allows access to VNC in libvirt
** Changed in: libvirt (Ubuntu Natty) Importance: High => Undecided ** Changed in: libvirt (Ubuntu Natty) Assignee: Serge Hallyn (serge-hallyn) => (unassigned) ** Changed in: qemu-kvm (Ubuntu Maverick) Milestone: maverick-updates => None ** Changed in: libvirt (Ubuntu Lucid) Status: New => Invalid -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/697197 Title: Empty password allows access to VNC in libvirt Status in libvirt virtualization API: Unknown Status in QEMU: Confirmed Status in qemu-kvm: Unknown Status in “libvirt” package in Ubuntu: Invalid Status in “qemu-kvm” package in Ubuntu: Fix Released Status in “libvirt” source package in Lucid: Invalid Status in “qemu-kvm” source package in Lucid: In Progress Status in “libvirt” source package in Maverick: Invalid Status in “qemu-kvm” source package in Maverick: In Progress Status in “libvirt” source package in Natty: Invalid Status in “qemu-kvm” source package in Natty: Fix Released Status in “libvirt” source package in Karmic: Invalid Status in “qemu-kvm” source package in Karmic: New Bug description: The help in the /etc/libvirt/qemu.conf states "To allow access without passwords, leave this commented out. An empty string will still enable passwords, but be rejected by QEMU effectively preventing any use of VNC." yet setting: vnc_password="" allows access to the vnc console without any password prompt just as if it is hashed out completely. ProblemType: Bug DistroRelease: Ubuntu 10.10 Package: libvirt-bin 0.8.3-1ubuntu14 ProcVersionSignature: Ubuntu 2.6.35-24.42-server 2.6.35.8 Uname: Linux 2.6.35-24-server x86_64 Architecture: amd64 Date: Tue Jan 4 12:18:35 2011 InstallationMedia: Ubuntu-Server 10.04.1 LTS "Lucid Lynx" - Release amd64 (20100816.2) ProcEnviron: LANG=en_GB.UTF-8 SHELL=/bin/bash SourcePackage: libvirt
[Qemu-devel] [Bug 697197] Re: Empty password allows access to VNC in libvirt
** Changed in: qemu-kvm (Ubuntu Maverick) Assignee: Ubuntu Security Team (ubuntu-security) => Kees Cook (kees) ** Changed in: qemu-kvm (Ubuntu Lucid) Assignee: Ubuntu Security Team (ubuntu-security) => Kees Cook (kees) ** Changed in: qemu-kvm (Ubuntu Karmic) Importance: Undecided => Medium ** Changed in: qemu-kvm (Ubuntu Karmic) Assignee: (unassigned) => Kees Cook (kees) ** Changed in: qemu-kvm (Ubuntu Lucid) Status: In Progress => Fix Committed ** Changed in: qemu-kvm (Ubuntu Maverick) Status: In Progress => Fix Committed ** Changed in: qemu-kvm (Ubuntu Karmic) Status: In Progress => Fix Committed -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/697197 Title: Empty password allows access to VNC in libvirt Status in libvirt virtualization API: Unknown Status in QEMU: Confirmed Status in qemu-kvm: Unknown Status in “libvirt” package in Ubuntu: Invalid Status in “qemu-kvm” package in Ubuntu: Fix Released Status in “libvirt” source package in Lucid: Invalid Status in “qemu-kvm” source package in Lucid: Fix Committed Status in “libvirt” source package in Maverick: Invalid Status in “qemu-kvm” source package in Maverick: Fix Committed Status in “libvirt” source package in Natty: Invalid Status in “qemu-kvm” source package in Natty: Fix Released Status in “libvirt” source package in Karmic: Invalid Status in “qemu-kvm” source package in Karmic: Fix Committed Bug description: The help in the /etc/libvirt/qemu.conf states "To allow access without passwords, leave this commented out. An empty string will still enable passwords, but be rejected by QEMU effectively preventing any use of VNC." yet setting: vnc_password="" allows access to the vnc console without any password prompt just as if it is hashed out completely. ProblemType: Bug DistroRelease: Ubuntu 10.10 Package: libvirt-bin 0.8.3-1ubuntu14 ProcVersionSignature: Ubuntu 2.6.35-24.42-server 2.6.35.8 Uname: Linux 2.6.35-24-server x86_64 Architecture: amd64 Date: Tue Jan 4 12:18:35 2011 InstallationMedia: Ubuntu-Server 10.04.1 LTS "Lucid Lynx" - Release amd64 (20100816.2) ProcEnviron: LANG=en_GB.UTF-8 SHELL=/bin/bash SourcePackage: libvirt
Re: [PATCH v1 5/9] KVM: x86: Add new hypercall to lock control registers
On Mon, May 29, 2023 at 06:48:03PM +0200, Mickaël Salaün wrote: > > On 08/05/2023 23:11, Wei Liu wrote: > > On Fri, May 05, 2023 at 05:20:42PM +0200, Mickaël Salaün wrote: > > > This enables guests to lock their CR0 and CR4 registers with a subset of > > > X86_CR0_WP, X86_CR4_SMEP, X86_CR4_SMAP, X86_CR4_UMIP, X86_CR4_FSGSBASE > > > and X86_CR4_CET flags. > > > > > > The new KVM_HC_LOCK_CR_UPDATE hypercall takes two arguments. The first > > > is to identify the control register, and the second is a bit mask to > > > pin (i.e. mark as read-only). > > > > > > These register flags should already be pinned by Linux guests, but once > > > compromised, this self-protection mechanism could be disabled, which is > > > not the case with this dedicated hypercall. > > > > > > Cc: Borislav Petkov > > > Cc: Dave Hansen > > > Cc: H. Peter Anvin > > > Cc: Ingo Molnar > > > Cc: Kees Cook > > > Cc: Madhavan T. Venkataraman > > > Cc: Paolo Bonzini > > > Cc: Sean Christopherson > > > Cc: Thomas Gleixner > > > Cc: Vitaly Kuznetsov > > > Cc: Wanpeng Li > > > Signed-off-by: Mickaël Salaün > > > Link: https://lore.kernel.org/r/20230505152046.6575-6-...@digikod.net > > [...] > > > hw_cr4 = (cr4_read_shadow() & X86_CR4_MCE) | (cr4 & > > > ~X86_CR4_MCE); > > > if (is_unrestricted_guest(vcpu)) > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > > index ffab64d08de3..a529455359ac 100644 > > > --- a/arch/x86/kvm/x86.c > > > +++ b/arch/x86/kvm/x86.c > > > @@ -7927,11 +7927,77 @@ static unsigned long emulator_get_cr(struct > > > x86_emulate_ctxt *ctxt, int cr) > > > return value; > > > } > > > +#ifdef CONFIG_HEKI > > > + > > > +extern unsigned long cr4_pinned_mask; > > > + > > > > Can this be moved to a header file? > > Yep, but I'm not sure which one. Any preference Kees? Uh, er, I was never expecting that mask to be non-static. ;) To that end, how about putting it in arch/x86/kvm/x86.h ? -- Kees Cook
Re: [Qemu-devel] [PATCH 1/3] virtio: Basic implementation of virtio pstore driver
On Sun, Sep 4, 2016 at 7:38 AM, Namhyung Kim wrote: > The virtio pstore driver provides interface to the pstore subsystem so > that the guest kernel's log/dump message can be saved on the host > machine. Users can access the log file directly on the host, or on the > guest at the next boot using pstore filesystem. It currently deals with > kernel log (printk) buffer only, but we can extend it to have other > information (like ftrace dump) later. > > It supports legacy PCI device using a 16K buffer by default and it's > configurable. It uses two virtqueues - one for (sync) read and another > for (async) write. Since it cannot wait for write finished, it supports > up to 128 concurrent IO. > > Cc: Paolo Bonzini > Cc: Radim Krčmář > Cc: "Michael S. Tsirkin" > Cc: Anthony Liguori > Cc: Anton Vorontsov > Cc: Colin Cross > Cc: Kees Cook > Cc: Tony Luck > Cc: Steven Rostedt > Cc: Ingo Molnar > Cc: Minchan Kim > Cc: Will Deacon > Cc: virtio-...@lists.oasis-open.org > Cc: k...@vger.kernel.org > Cc: qemu-devel@nongnu.org > Cc: virtualizat...@lists.linux-foundation.org > Signed-off-by: Namhyung Kim While I can't speak to the virtio parts, the interface into pstore looks fine to me. :) Reviewed-by: Kees Cook -Kees -- Kees Cook Nexus Security
Re: [Qemu-devel] [PATCH 1/3] virtio: Basic implementation of virtio pstore driver
On Sun, Jul 17, 2016 at 9:37 PM, Namhyung Kim wrote:
> The virtio pstore driver provides interface to the pstore subsystem so
> that the guest kernel's log/dump message can be saved on the host
> machine. Users can access the log file directly on the host, or on the
> guest at the next boot using pstore filesystem. It currently deals with
> kernel log (printk) buffer only, but we can extend it to have other
> information (like ftrace dump) later.
>
> It supports legacy PCI device using single order-2 page buffer. As all
> operation of pstore is synchronous, it would be fine IMHO. However I
> don't know how to make write operation synchronous since it's called
> with a spinlock held (from any context including NMI).
>
> Cc: Paolo Bonzini
> Cc: Radim Krčmář
> Cc: "Michael S. Tsirkin"
> Cc: Anthony Liguori
> Cc: Anton Vorontsov
> Cc: Colin Cross
> Cc: Kees Cook
> Cc: Tony Luck
> Cc: Steven Rostedt
> Cc: Ingo Molnar
> Cc: Minchan Kim
> Cc: k...@vger.kernel.org
> Cc: qemu-devel@nongnu.org
> Cc: virtualizat...@lists.linux-foundation.org
> Signed-off-by: Namhyung Kim
This looks great to me! I'd love to use this in qemu. (Right now I go
through hoops to use the ramoops backend for testing.)
Reviewed-by: Kees Cook
Notes below...
> ---
> drivers/virtio/Kconfig | 10 ++
> drivers/virtio/Makefile| 1 +
> drivers/virtio/virtio_pstore.c | 317
> +
> include/uapi/linux/Kbuild | 1 +
> include/uapi/linux/virtio_ids.h| 1 +
> include/uapi/linux/virtio_pstore.h | 53 +++
> 6 files changed, 383 insertions(+)
> create mode 100644 drivers/virtio/virtio_pstore.c
> create mode 100644 include/uapi/linux/virtio_pstore.h
>
> diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig
> index 77590320d44c..8f0e6c796c12 100644
> --- a/drivers/virtio/Kconfig
> +++ b/drivers/virtio/Kconfig
> @@ -58,6 +58,16 @@ config VIRTIO_INPUT
>
> If unsure, say M.
>
> +config VIRTIO_PSTORE
> + tristate "Virtio pstore driver"
> + depends on VIRTIO
> + depends on PSTORE
> + ---help---
> +This driver supports virtio pstore devices to save/restore
> +panic and oops messages on the host.
> +
> +If unsure, say M.
> +
> config VIRTIO_MMIO
> tristate "Platform bus driver for memory mapped virtio devices"
> depends on HAS_IOMEM && HAS_DMA
> diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile
> index 41e30e3dc842..bee68cb26d48 100644
> --- a/drivers/virtio/Makefile
> +++ b/drivers/virtio/Makefile
> @@ -5,3 +5,4 @@ virtio_pci-y := virtio_pci_modern.o virtio_pci_common.o
> virtio_pci-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o
> obj-$(CONFIG_VIRTIO_BALLOON) += virtio_balloon.o
> obj-$(CONFIG_VIRTIO_INPUT) += virtio_input.o
> +obj-$(CONFIG_VIRTIO_PSTORE) += virtio_pstore.o
> diff --git a/drivers/virtio/virtio_pstore.c b/drivers/virtio/virtio_pstore.c
> new file mode 100644
> index ..6fe62c0f1508
> --- /dev/null
> +++ b/drivers/virtio/virtio_pstore.c
> @@ -0,0 +1,317 @@
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define VIRT_PSTORE_ORDER2
> +#define VIRT_PSTORE_BUFSIZE (4096 << VIRT_PSTORE_ORDER)
> +
> +struct virtio_pstore {
> + struct virtio_device*vdev;
> + struct virtqueue*vq;
> + struct pstore_info pstore;
> + struct virtio_pstore_hdr hdr;
> + size_t buflen;
> + u64 id;
> +
> + /* Waiting for host to ack */
> + wait_queue_head_t acked;
> +};
> +
> +static u16 to_virtio_type(struct virtio_pstore *vps, enum pstore_type_id
> type)
> +{
> + u16 ret;
> +
> + switch (type) {
> + case PSTORE_TYPE_DMESG:
> + ret = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_DMESG);
> + break;
> + default:
> + ret = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_UNKNOWN);
> + break;
> + }
I would love to see this support PSTORE_TYPE_CONSOLE too. It should be
relatively easy to add: I think it'd just be another virtio command?
> +
> + return ret;
> +}
> +
> +static enum pstore_type_id from_virtio_type(struct virtio_pstore *vps, u16
> type)
> +{
> + enum pstore_type_id ret;
> +
> + switch (virtio16_to_cpu(vps->vdev, type)) {
> + case VIRTIO_PSTORE_TYPE_DMESG:
> +
Re: [Qemu-devel] [PATCH 1/3] virtio: Basic implementation of virtio pstore driver
On Sun, Jul 17, 2016 at 10:50 PM, Namhyung Kim wrote:
> Hello,
>
> On Sun, Jul 17, 2016 at 10:12:26PM -0700, Kees Cook wrote:
>> On Sun, Jul 17, 2016 at 9:37 PM, Namhyung Kim wrote:
>> > The virtio pstore driver provides interface to the pstore subsystem so
>> > that the guest kernel's log/dump message can be saved on the host
>> > machine. Users can access the log file directly on the host, or on the
>> > guest at the next boot using pstore filesystem. It currently deals with
>> > kernel log (printk) buffer only, but we can extend it to have other
>> > information (like ftrace dump) later.
>> >
>> > It supports legacy PCI device using single order-2 page buffer. As all
>> > operation of pstore is synchronous, it would be fine IMHO. However I
>> > don't know how to make write operation synchronous since it's called
>> > with a spinlock held (from any context including NMI).
>> >
>> > Cc: Paolo Bonzini
>> > Cc: Radim Kr??m
>> > Cc: "Michael S. Tsirkin"
>> > Cc: Anthony Liguori
>> > Cc: Anton Vorontsov
>> > Cc: Colin Cross
>> > Cc: Kees Cook
>> > Cc: Tony Luck
>> > Cc: Steven Rostedt
>> > Cc: Ingo Molnar
>> > Cc: Minchan Kim
>> > Cc: k...@vger.kernel.org
>> > Cc: qemu-devel@nongnu.org
>> > Cc: virtualizat...@lists.linux-foundation.org
>> > Signed-off-by: Namhyung Kim
>>
>> This looks great to me! I'd love to use this in qemu. (Right now I go
>> through hoops to use the ramoops backend for testing.)
>>
>> Reviewed-by: Kees Cook
>
> Thank you!
>
>>
>> Notes below...
>>
>
> [SNIP]
>> > +static u16 to_virtio_type(struct virtio_pstore *vps, enum pstore_type_id
>> > type)
>> > +{
>> > + u16 ret;
>> > +
>> > + switch (type) {
>> > + case PSTORE_TYPE_DMESG:
>> > + ret = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_TYPE_DMESG);
>> > + break;
>> > + default:
>> > + ret = cpu_to_virtio16(vps->vdev,
>> > VIRTIO_PSTORE_TYPE_UNKNOWN);
>> > + break;
>> > + }
>>
>> I would love to see this support PSTORE_TYPE_CONSOLE too. It should be
>> relatively easy to add: I think it'd just be another virtio command?
>
> Do you want to append the data to the host file as guest does
> printk()? I think it needs some kind of buffer management, but it's
> not hard to add IMHO.
Well, with most pstore backends, the buffer size is limited, so it
tends to be a circular buffer of some sort. I think whatever you
choose to do is fine (I saw the various mentions of resource limits in
the qemu part of this thread), as long as the last N bytes of console
can be seen on the host side, where N is some portion of the memory
set aside for the log. (I don't mind the idea of an unlimited console
log either, but I suspect that will not be accepted on the qemu
side...)
> [SNIP]
>> > +static int notrace virt_pstore_write(enum pstore_type_id type,
>> > +enum kmsg_dump_reason reason,
>> > +u64 *id, unsigned int part, int count,
>> > +bool compressed, size_t size,
>> > +struct pstore_info *psi)
>> > +{
>> > + struct virtio_pstore *vps = psi->data;
>> > + struct virtio_pstore_hdr *hdr = &vps->hdr;
>> > + struct scatterlist sg[2];
>> > + unsigned int flags = compressed ? VIRTIO_PSTORE_FL_COMPRESSED : 0;
>> > +
>> > + *id = vps->id++;
>> > +
>> > + hdr->cmd = cpu_to_virtio16(vps->vdev, VIRTIO_PSTORE_CMD_WRITE);
>> > + hdr->id= cpu_to_virtio64(vps->vdev, *id);
>> > + hdr->flags = cpu_to_virtio32(vps->vdev, flags);
>> > + hdr->type = to_virtio_type(vps, type);
>> > +
>> > + sg_init_table(sg, 2);
>> > + sg_set_buf(&sg[0], hdr, sizeof(*hdr));
>> > + sg_set_buf(&sg[1], psi->buf, size);
>> > + virtqueue_add_outbuf(vps->vq, sg, 2, vps, GFP_ATOMIC);
>> > + virtqueue_kick(vps->vq);
>> > +
>> > + /* TODO: make it synchronous */
>> > + return 0;
>>
>> The down side to this being asynchronous is the lack of error
>> reporting. Perhaps this could check hdr->type before queuing and error
>> for any
[Qemu-devel] [PATCH] nvdimm: Add docs hint for Linux driver name
I spent way too much time trying to figure out why the emulated NVDIMM was missing under Linux. In an effort to help others who might be looking for these kinds of things in the future, include a hint. Signed-off-by: Kees Cook --- docs/nvdimm.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt index 5f158a61708e..7231c2d78f65 100644 --- a/docs/nvdimm.txt +++ b/docs/nvdimm.txt @@ -49,8 +49,9 @@ Multiple vNVDIMM devices can be created if multiple pairs of "-object" and "-device" are provided. For above command line options, if the guest OS has the proper NVDIMM -driver, it should be able to detect a NVDIMM device which is in the -persistent memory mode and whose size is $NVDIMM_SIZE. +driver (e.g. "CONFIG_ACPI_NFIT=y" under Linux), it should be able to +detect a NVDIMM device which is in the persistent memory mode and whose +size is $NVDIMM_SIZE. Note: -- 2.17.1 -- Kees Cook Pixel Security
Re: [PATCH] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute
On Fri, Apr 02, 2021 at 08:42:07AM +0200, Sedat Dilek wrote: > On Thu, Feb 25, 2021 at 10:25 PM Kees Cook wrote: > > > > On Thu, 11 Feb 2021 12:42:58 -0700, Nathan Chancellor wrote: > > > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > > > which violates clang's CFI checking because fw_cfg_showrev()'s second > > > parameter is 'struct attribute', whereas the ->show() member of 'struct > > > kobj_structure' expects the second parameter to be of type 'struct > > > kobj_attribute'. > > > > > > $ cat /sys/firmware/qemu_fw_cfg/rev > > > 3 > > > > > > [...] > > > > Applied to kspp/cfi/cleanups, thanks! > > > > [1/1] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute > > https://git.kernel.org/kees/c/f5c4679d6c49 > > > > I have queued this up in my custom patchset > (for-5.12/kspp-cfi-cleanups-20210225). > > What is the plan to get this upstream? I haven't sent it to Linus yet -- I was expecting to batch more of these and send them for v5.13. (But if the kvm folks snag it, that's good too.) -Kees > > Feel free to add my: > > Tested-by: Sedat Dilek > > - Sedat - -- Kees Cook
Re: [PATCH] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute
On Thu, Feb 11, 2021 at 12:42:58PM -0700, Nathan Chancellor wrote:
> fw_cfg_showrev() is called by an indirect call in kobj_attr_show(),
> which violates clang's CFI checking because fw_cfg_showrev()'s second
> parameter is 'struct attribute', whereas the ->show() member of 'struct
> kobj_structure' expects the second parameter to be of type 'struct
> kobj_attribute'.
>
> $ cat /sys/firmware/qemu_fw_cfg/rev
> 3
>
> $ dmesg | grep "CFI failure"
> [ 26.016832] CFI failure (target: fw_cfg_showrev+0x0/0x8):
>
> Fix this by converting fw_cfg_rev_attr to 'struct kobj_attribute' where
> this would have been caught automatically by the incompatible pointer
> types compiler warning. Update fw_cfg_showrev() accordingly.
>
> Fixes: 75f3e8e47f38 ("firmware: introduce sysfs driver for QEMU's fw_cfg
> device")
> Link: https://github.com/ClangBuiltLinux/linux/issues/1299
> Signed-off-by: Nathan Chancellor
Ah, nice, yes.
Reviewed-by: Kees Cook
Michael, are you able to take this? I can snag it if needed.
-Kees
> ---
> drivers/firmware/qemu_fw_cfg.c | 8 +++-
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/firmware/qemu_fw_cfg.c b/drivers/firmware/qemu_fw_cfg.c
> index 0078260fbabe..172c751a4f6c 100644
> --- a/drivers/firmware/qemu_fw_cfg.c
> +++ b/drivers/firmware/qemu_fw_cfg.c
> @@ -299,15 +299,13 @@ static int fw_cfg_do_platform_probe(struct
> platform_device *pdev)
> return 0;
> }
>
> -static ssize_t fw_cfg_showrev(struct kobject *k, struct attribute *a, char
> *buf)
> +static ssize_t fw_cfg_showrev(struct kobject *k, struct kobj_attribute *a,
> + char *buf)
> {
> return sprintf(buf, "%u\n", fw_cfg_rev);
> }
>
> -static const struct {
> - struct attribute attr;
> - ssize_t (*show)(struct kobject *k, struct attribute *a, char *buf);
> -} fw_cfg_rev_attr = {
> +static const struct kobj_attribute fw_cfg_rev_attr = {
> .attr = { .name = "rev", .mode = S_IRUSR },
> .show = fw_cfg_showrev,
> };
>
> base-commit: 92bf22614b21a2706f4993b278017e437f7785b3
> --
> 2.30.1
>
--
Kees Cook
Re: [PATCH] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute
On Thu, 11 Feb 2021 12:42:58 -0700, Nathan Chancellor wrote: > fw_cfg_showrev() is called by an indirect call in kobj_attr_show(), > which violates clang's CFI checking because fw_cfg_showrev()'s second > parameter is 'struct attribute', whereas the ->show() member of 'struct > kobj_structure' expects the second parameter to be of type 'struct > kobj_attribute'. > > $ cat /sys/firmware/qemu_fw_cfg/rev > 3 > > [...] Applied to kspp/cfi/cleanups, thanks! [1/1] qemu_fw_cfg: Make fw_cfg_rev_attr a proper kobj_attribute https://git.kernel.org/kees/c/f5c4679d6c49 -- Kees Cook
