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]

Message-ID: <CAHk-=wggnfCR2JcC-U9LxfeBo2UMagd-neEs8PwDHsGVfLfS=Q@mail.gmail.com>
Date:   Tue, 25 Feb 2020 10:08:51 -0800
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Denis Efremov <efremov@...ux.com>
Cc:     Willy Tarreau <w@....eu>, 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 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.

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

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.

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

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.

                Linus