linux-kernel - Re: [PATCH] Bluetooth: hci_h5: implement CRC data integrity

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] [day] [month] [year] [list]

Message-ID: <CABBYNZLqcdNTh_aMe3JVKBLauNkXq=6n+xTt7q798cDkZGaO9Q@mail.gmail.com>
Date: Mon, 29 Sep 2025 12:01:41 -0400
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Javier Nieto <jgnieto@...stanford.edu>
Cc: Paul Menzel <pmenzel@...gen.mpg.de>, marcel@...tmann.org, 
	linux-bluetooth@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Bluetooth: hci_h5: implement CRC data integrity

Hi Javier,

On Sun, Aug 31, 2025 at 10:20 PM Javier Nieto <jgnieto@...stanford.edu> wrote:
>
> Dear Paul,
>
> Thanks for the review!
>
> On Wed, Aug 27, 2025 at 12:56:50PM +0200, Paul Menzel wrote:
>
> > Any btmon trace?
>
> The presence of CRC is limited to the H5 layer, so it is not visible on
> btmon. However, I did advertise and connect to a few devices while
> running btmon and everything worked and looked as normal. I also ensured
> that CRC was being used by adding temporary debugging prints.
>
> > I´d add the above to the proper commit message.
>
> Should I resubmit the patch as v2?
>
> > >   static u8 h5_cfg_field(struct h5 *h5)
> > >   {
> > > -   /* Sliding window size (first 3 bits) */
> > > -   return h5->tx_win & 0x07;
> > > +   /* Sliding window size (first 3 bits) and CRC request (fifth bit). */
> > > +   return (h5->tx_win & 0x07) | 0x10;
> >
> > Could a macro be defined for the CRC request bit?
>
> I thought about this, but decided against it since 0x10 is only used
> here and in one other place. Also, the existing code does not define a
> macro for the window size bits 0x07. I am not opposed to adding it if
> someone feels strongly about it though.
>
> > The diff looks good. Feel free to carry:
> >
> > Reviewed-by: Paul Menzel <pmenzel@...gen.mpg.de>
> >
> >
> > Kind regards,
> >
> > Paul
>
> I see that my patch fails a few test cases because it fails to link
> crc-ccitt. Do you know whether this is a problem with my patch or the
> test environment and where the code for the tests is found?

Yeah, this seems really a problem you are introducing:

ld: vmlinux.o: in function `h5_rx_crc':
hci_h5.c:(.text+0xd938d9): undefined reference to `crc_ccitt'
ld: vmlinux.o: in function `h5_prepare_pkt':
hci_h5.c:(.text+0xd94798): undefined reference to `crc_ccitt'
ld: hci_h5.c:(.text+0xd947a6): undefined reference to `crc_ccitt'

I guess we are missing something like select CRC_CCITT on Kconfig.

> Thanks again for your feedback.
>
> Javier



-- 
Luiz Augusto von Dentz