linux-kernel - Re: [PATCH] tty: Revert the removal of the Cyclades public API

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: <alpine.DEB.2.20.2201171827130.11348@tpp.orcam.me.uk>
Date:   Sun, 23 Jan 2022 13:32:21 +0000 (GMT)
From:   "Maciej W. Rozycki" <macro@...ecosm.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
cc:     Jiri Slaby <jirislaby@...nel.org>, linux-serial@...r.kernel.org,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH] tty: Revert the removal of the Cyclades public API

On Mon, 17 Jan 2022, Greg Kroah-Hartman wrote:

> >  Because they have become a part of the published API.  Someone may even 
> > use a system using headers from the most recent version of the Linux 
> > kernel (though not necessarily running such a kernel) to build software 
> > intended to run on an older version that still does implement the API.  
> > Times where people individually built pefectly matching software from 
> > sources to run on each system they looked after have largely long gone.
> 
> For hardware-specific things like this, it's not the same.  I can see
> adding the .h file as empty just to keep things building, but if no one
> is actually ever using the structures and definitions in the file, it
> should stay removed.
> 
> In looking at the file itself, it just looks like it wants a single
> structure, struct cyclades_monitor, and then never actually does
> anything with it.

 According to the error messages I got when I added the missing structure 
only it refers a number of ioctls as well, clearly making use of them 
somehow:

.../libsanitizer/sanitizer_common/sanitizer_platform_limits_posix.cc:836:35: error: 'CYGETDEFTHRESH' was not declared in this scope; did you mean 'IOCTL_CYGETDEFTHRESH'?
  836 |   unsigned IOCTL_CYGETDEFTHRESH = CYGETDEFTHRESH;
      |                                   ^~~~~~~~~~~~~~
      |                                   IOCTL_CYGETDEFTHRESH

etc.

> So I guess I should submit a patch to the llvm developers to remove
> these lines and add back the single structure definition to allow this
> to keep building now?

 Side note: I've encountered it with libsanitizer bundled with GCC rather 
than LLVM; I don't know what use of libsanitizer LLVM makes or what their 
maintenance/release policies are.

 You can't add anything back to something that has been released long ago, 
e.g. <ftp://ftp.gnu.org/gnu/gcc/gcc-9.1.0/gcc-9.1.0.tar.gz>.  OK, it's 
less than 3 years ago, so not very long really, but the same applies: that 
release has been cast in stone and `gcc-9.1.0.tar.gz' will be as it is 
forever.  A user will expect to just download it and successfully build 
with their system, be it now or in 10 years' time.

 Of course reality may vary, but that is only supposed to happen because 
people make mistakes and not because they deliberately and unilaterally 
terminate a contract such as an API is.

> >  Well, they have been exported, so they have become a part of the API.  
> > This user program may not use them, another one will.  If you don't want 
> > an API to become public, then do not export it in the first place.
> 
> That happened a very long time ago, for hardware that no one has, sorry.
> 
> So the "ABI" broke when the driver was removed.  Given that no one has
> the hardware, no one noticed the breakage, so there is no breakage :)

 The ABI is still there, that is if a binary that has been built according 
to this API tries to use it the kernel with the driver obsoleted won't do 
anything unexpected.  It will of course return some kind of an error, but 
returning errors has been a part of the API and therefore any sane program 
must have been prepared to handle anyway (e.g. driver not configured in, 
hardware not present, device on fire, etc.).

> >  So it shouldn't have been a part of the user API in the first place.  
> > Given that it has become a part of it it has to stay, that's the whole 
> > point of having a user API.
> 
> But what user program uses that value?  I can't seem to find any, so
> pointers would be appreciated.

 Well, an API is an API.  A contract as I pointed out.  Such a program 
need not be publicly available and we may not be able to trace it.

> I'll gladly take a patch that just adds the one needed structure to keep
> this file building.  But that's all that is needed unless someone can
> point out other code that needs these definitions.

 Well, I don't feel like arguing even though I don't think it's the right 
approach, so taking your word for acceptance I'm sending v2 adjusted to 
your requirements right away.

  Maciej