linux-kernel - Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS

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:   Tue, 25 Feb 2020 19:15:41 +0100
From:   Willy Tarreau <w@....eu>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Denis Efremov <efremov@...ux.com>, Jens Axboe <axboe@...nel.dk>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-block <linux-block@...r.kernel.org>
Subject: Re: [PATCH 01/10] floppy: cleanup: expand macro FDCS

On Tue, Feb 25, 2020 at 10:08:51AM -0800, Linus Torvalds wrote:
> On Tue, Feb 25, 2020 at 7:22 AM Denis Efremov <efremov@...ux.com> wrote:
> >
> > I think that for the first attempt changing will be enough:
> > -static int fdc;                        /* current fdc */
> > +static int current_fdc;                        /* current fdc */
> > and
> > -#define FD_IOPORT fdc_state[fdc].address
> > +#define FD_IOPORT fdc_state[current_fdc].address
> 
> Please don't do this blindly - ie without verifying that there are no
> cases of that "local fdc variable shadowing" issue.

That's exactly what I'm doing. In fact I first renamed the variable
and am manually checking all places which do not compile anymore.
Hence the surprizes.

> Of course, such a verification might be as easy as "generates exact
> same code" rather than looking at every use.

That's exactly what I'm doing.

> And btw, don't worry too much about this being in an UAPI file. I'm
> pretty sure that's because of specialty programs that use the magical
> ioctls to do special formatting. They want the special commands
> (FD_FORMAT etc), but I don't think they really use the port addresses.
> 
> So I think it's in a UAPI file entirely by mistake.

OK this will help me, thanks for the hint :-)

> We should at least try moving those bits to the floppy.c file and
> remove it from the header file.

Makes sense.

> For example, doing a Debian code search on "FDPATCHES" doesn't find
> any user space hits. Searching for "FD_STATUS" gets a lot of hits, but
> thos all seem to be because it's a symbol used by user space programs,
> ("file descriptor status"), not because those hits actually used the
> fdreg.h header file.
> 
> So we can remove at least the FD_IOPORT mess from the header file, I bet.
> 
> Worst case - if somebody finds some case that uses them, we can put it back.

I like that. And at least we'll know how they use it (likely without the
dependency on fdc).

Thanks,
Willy