linux-kernel - Re: [PATCH RESEND 1/1] check-uapi: Introduce check-uapi.sh

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]

Message-ID: <ca30cded-4ea4-8c70-61ad-5d0d99b123d3@quicinc.com>
Date:   Sat, 18 Feb 2023 10:22:09 -0800
From:   Trilok Soni <quic_tsoni@...cinc.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        John Moon <quic_johmoo@...cinc.com>,
        Randy Dunlap <rdunlap@...radead.org>
CC:     Masahiro Yamada <masahiroy@...nel.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Nick Desaulniers <ndesaulniers@...gle.com>,
        "Nicolas Schier" <nicolas@...sle.eu>,
        <linux-kbuild@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...r.kernel.org>,
        <linux-arm-msm@...r.kernel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        "Todd Kjos" <tkjos@...gle.com>,
        Matthias Maennich <maennich@...gle.com>,
        "Giuliano Procida" <gprocida@...gle.com>,
        <kernel-team@...roid.com>, Jordan Crouse <jorcrous@...zon.com>
Subject: Re: [PATCH RESEND 1/1] check-uapi: Introduce check-uapi.sh

On 2/18/2023 12:31 AM, Greg Kroah-Hartman wrote:
> On Sat, Feb 18, 2023 at 09:17:12AM +0100, Greg Kroah-Hartman wrote:
>> On Fri, Feb 17, 2023 at 12:22:34PM -0800, John Moon wrote:
>>> While the kernel community has been good at maintaining backwards
>>> compatibility with kernel UAPIs, it would be helpful to have a tool
>>> to check if a patch introduces changes that break backwards
>>> compatibility.
>>>
>>> To that end, introduce check-uapi.sh: a simple shell script that
>>> checks for changes to UAPI headers using libabigail.
>>>
>>> libabigail is "a framework which aims at helping developers and
>>> software distributors to spot some ABI-related issues like interface
>>> incompatibility in ELF shared libraries by performing a static
>>> analysis of the ELF binaries at hand."
>>>
>>> The script uses one of libabigail's tools, "abidiff", to compile the
>>> changed header before and after the patch to detect any changes.
>>>
>>> abidiff "compares the ABI of two shared libraries in ELF format. It
>>> emits a meaningful report describing the differences between the two
>>> ABIs."
>>>
>>> Signed-off-by: John Moon <quic_johmoo@...cinc.com>
>>> ---
>>>   scripts/check-uapi.sh | 245 ++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 245 insertions(+)
>>>   create mode 100755 scripts/check-uapi.sh
>>
>> Ok, this is very cool, thank you so much for doing this.
>>
>> I know Randy Dunlap was also looking into this previously, so I've cc:ed
>> him and bounced him the original.
>>
>> I tried this out, and at first glance, this felt like it was just "too
>> fast" in that nothing actually was being tested.  So I manually added a
>> field to a structure I know would break the abi, and:
>>
>> 	$ ./scripts/check-uapi.sh
>> 	!!! ABI differences detected in include/uapi/linux/usb/ch9.h (compared to file at HEAD^1) !!!
>>
>> 	    [C] 'struct usb_ctrlrequest' changed:
>> 	      type size changed from 64 to 72 (in bits)
>> 	      1 data member insertion:
>> 		'__u8 abi_break', at offset 16 (in bits) at ch9.h:216:1
>> 	      3 data member changes:
>> 		'__le16 wValue' offset changed from 16 to 24 (in bits) (by +8 bits)
>> 		'__le16 wIndex' offset changed from 32 to 40 (in bits) (by +8 bits)
>> 		'__le16 wLength' offset changed from 48 to 56 (in bits) (by +8 bits)
>>
>> 	0/1 UAPI header file changes are backwards compatible
>> 	UAPI header ABI check failed
>>
>> So it worked!
> 
> Ok, I take it back, it doesn't seem to work :(
> 
> It only "catches" a change from the last commit, but if you have an
> intermediate commit (i.e change something in HEAD^ but not HEAD), it
> does not detect it at all.
> 
> And if you give it an old version, it doesn't check that either (hint,
> try passing in a very old kernel version, that returns instantly and
> doesn't actually build anything).
> 
> So it's a good first cut as an example, but as it doesn't really work
> correctly yet, we can't take it.  Care to fix it up to work so that it
> can be usable?

These first patches were to introduce the tool w/ the one scenario only, 
and thanks for trying it out. We can fix it and add your suggestions.

---Trilok Soni