linux-kernel - Re: [Patch V2 08/13] genirq: Set auxiliary data for an interrupt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 26 Mar 2021 16:09:11 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Megha Dey <megha.dey@...el.com>, linux-kernel@...r.kernel.org,
        dave.jiang@...el.com, ashok.raj@...el.com, kevin.tian@...el.com,
        dwmw@...zon.co.uk, x86@...nel.org, tony.luck@...el.com,
        dan.j.williams@...el.com, jgg@...lanox.com, kvm@...r.kernel.org,
        iommu@...ts.linux-foundation.org, alex.williamson@...hat.com,
        bhelgaas@...gle.com, linux-pci@...r.kernel.org,
        baolu.lu@...ux.intel.com, ravi.v.shankar@...el.com
Subject: Re: [Patch V2 08/13] genirq: Set auxiliary data for an interrupt

On Fri, Mar 26 2021 at 10:32, Marc Zyngier wrote:
> On Thu, 25 Mar 2021 18:59:48 +0000,
> Thomas Gleixner <tglx@...utronix.de> wrote:
>> Though that leaves the question of the data type for 'val'. While u64 is
>> probably good enough for most stuff, anything which needs more than that
>> is left out (again). union as arguments are horrible especially if you
>> need the counterpart irq_get_auxdata() for which you need a pointer and
>> then you can't do a forward declaration. Something like this might work
>> though and avoid to make the pointer business unconditional:
>> 
>>         struct irq_auxdata {
>>                union {
>>         	     u64        val;
>>                      struct foo *foo;
>>                };
>>         };
>
> I guess that at some point, irq_get_auxdata() will become a
> requirement so we might as well bite the bullet now, and the above
> looks like a good start to me.

And because it's some time until rustification, we can come up with
something nasty like the below:

#define IRQ_AUXTYPE(t, m)       IRQ_AUXTYPE_ ## t

enum irq_auxdata_type {
        IRQ_AUXTYPE(U64,		val64),
        IRQ_AUXTYPE(IRQSTATE,		istate),
        IRQ_AUXTYPE(VCPU_AFFINITY,	vaff),
};

struct irq_auxdata {
        union {
                u64             val64;
                u8              istate;
                struct vcpu_aff *vaff;
       };
};

This does not protect you from accessing the wrong union member, but it
allows an analyzer to map IRQ_AUXTYPE_U64 to irq_auxdata::val64 and then
check both the call site and the implementation whether they fiddle with
some other member of irq_auxdata or do weird casts.

Maybe it's just nuts and has no value, but I had the urge to write some
nasty macros.

Thanks,

        tglx