linux-kernel - RE: Build fail in drivers/gpu/drm/i915/display/intel

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]

Date:   Wed, 15 Nov 2023 09:26:10 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     "'imre.deak@...el.com'" <imre.deak@...el.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: Build fail in drivers/gpu/drm/i915/display/intel_tc.c

From: Imre Deak
> Sent: 14 November 2023 19:08
> 
> On Fri, Nov 10, 2023 at 09:00:21AM +0000, David Laight wrote:
> > From: Linus Torvalds
> > > Sent: 10 November 2023 00:52
> > >
> > > On Thu, 9 Nov 2023 at 15:34, Imre Deak <imre.deak@...el.com> wrote:
> > > >
> > > > The compiler warn should be fixed/suppressed by:
> > > > https://lore.kernel.org/all/20231026125636.5080-1-nirmoy.das@intel.com
> > >
> > > Ugh, so now it's a dynamic allocation, wasting memory, and a pointer
> > > to it, using as much memory as the array did in the first place.
> > >
> > > All because of a pointless warning that was a false positive - and was
> > > always harmless anyway, since snprintf() is safe (ie it was only a
> > > "might be truncated").
> >
> > That entire warning for snprintf() is a false positive.
> > The ones that are likely to overflow unexpectedly are the ones
> > with a "%s" format for a 'char *' pointer where there is no
> > implied length.
> >
> > The same check for printf() using the implied buffer length
> > probably does make sense.
> >
> > I don't even think there is a way of avoiding it on a case by case
> > basis - apart from passing both the buffer address and length
> > to an inline asm that the compiler has to assume might change
> > the values, but that tends to generate an extra 'mov' instruction
> > for no good reason at all.
> >
> > >
> > > Please don't do this. Either do that ((tc_port & 7) + 1) suggestion of
> > > David's, or just do '%c' and make the expression be
> > >
> > >   '1' + tc_port
> > >
> > > which should be fine since I915_MAX_PORTS is 8 or whatever.
> 
> Ok, the patch above was merged already to drm-tip,

I'd noticed ....

> but I agree not to use kasprintf for this.
> The above looks ok and there is already
> tc_port_name() for this, would just need to export that from
> intel_display.h.

I'm not sure that #define does anything except obscure what
is going on.
Anyone reading the code is going to search for it and then sigh.
But it is your code :-)

> I can follow up with a patch for this, or if you or David wanted to do
> that please send a patch to the intel-gfx@...ts.freedesktop.org list.

Probably much easier for you to do it.
My process to get patches emailed is a PITA.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)