linux-hardening - Re: [PATCH] tty: mxser: Remove __counted_by from mxser

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: <20240530174326.GB3018978@thelio-3990X>
Date: Thu, 30 May 2024 10:43:26 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jirislaby@...nel.org>
Cc: Kees Cook <keescook@...omium.org>,
	"Gustavo A. R. Silva" <gustavoars@...nel.org>,
	Bill Wendling <morbo@...gle.com>,
	Justin Stitt <justinstitt@...gle.com>, linux-serial@...r.kernel.org,
	imx@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
	linux-hardening@...r.kernel.org, llvm@...ts.linux.dev,
	patches@...ts.linux.dev, stable@...r.kernel.org
Subject: Re: [PATCH] tty: mxser: Remove __counted_by from mxser_board.ports[]

On Thu, May 30, 2024 at 09:40:13AM +0200, Greg Kroah-Hartman wrote:
> On Thu, May 30, 2024 at 08:22:03AM +0200, Jiri Slaby wrote:
> > >  This will be an error in a future compiler version [-Werror,-Wbounds-safety-counted-by-elt-type-unknown-size]
> > >      291 |         struct mxser_port ports[] __counted_by(nports);
> > >          |         ^~~~~~~~~~~~~~~~~~~~~~~~~
> > >    1 error generated.
> > > 
> > > Remove this use of __counted_by to fix the warning/error. However,
> > > rather than remove it altogether, leave it commented, as it may be
> > > possible to support this in future compiler releases.
> > 
> > This looks like a compiler bug/deficiency.

I apologize. As I commented on the nvmet-fc patch, I should have
included where the flexible array member was within 'struct mxser_port',
especially since I already knew it from
https://github.com/ClangBuiltLinux/linux/issues/2026.

I hope we can all agree now that this is not a compiler bug or issue.

> I agree, why not just turn that option off in the compiler so that these
> "warnings" will not show up?

I think the only part of the warning text that got quoted above should
clarify why this is not a real solution.

For the record, if this was a true issue on the compiler side, I would
have made that very clear in the commit message (or perhaps not even
sent the patch in the first place and worked to get it fixed on the
compiler side, as ClangBuiltLinux has always tried to do first since the
beginning).

> > What does gcc say BTW?

GCC does not have any support for __counted_by merged yet. I suspect
that its current implementation won't say anything either, otherwise
Kees should have noticed it in his testing. As you and Gustavo note
further down thread, sentinel is flagged by
-Wflex-array-member-not-at-end.

  In file included from include/linux/tty_port.h:8,
                   from include/linux/tty.h:11,
                   from drivers/tty/mxser.c:24:
  include/linux/tty_buffer.h:40:27: warning: structure containing a flexible array member is not at the end of another structure [-Wflex-array-member-not-at-end]
     40 |         struct tty_buffer sentinel;
        |                           ^~~~~~~~

Gustavo has a suggested diff to resolve the issue at the GitHub issue I
linked above that seems like a reasonable fix for both issues that is
small enough to go into stable trees that contain f34907ecca71 like this
one?

Cheers,
Nathan