Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <9eeb781a2d04d8fdef41611674ed91029382885d.camel@sipsolutions.net>
Subject: Re: [PATCH] rfkill: keep rfkill event compatibility with old
 userspace applications
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Jose Ignacio Tornos Martinez <jtornosm@redhat.com>,
        linux-wireless@vger.kernel.org
Date:   Tue, 15 Mar 2022 11:49:34 +0100
In-Reply-To: <20220315102641.233703-1-jtornosm@redhat.com>
References: <20220315102641.233703-1-jtornosm@redhat.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.4 (3.42.4-1.fc35) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 2022-03-15 at 11:26 +0100, Jose Ignacio Tornos Martinez wrote:
> Old userspace applications (for example bluez version before c939747f543a),
> that still use the original format for rfkill events (with 8 bytes size /
> RFKILL_EVENT_SIZE_V1) and are not requesting any specific size but a large
> one, are broken because they are checking the received size.

... because they're *not* checking the received size.

g-s-d by the way is even more broken before the fixes, because it
assumes that this thing is a stream protocol, rather than individual
messages. This was never OK, it just happened to work anyway.

> In order to avoid this compatibility issue, we can try to adapt by checking
> specific unusual requested sizes:
> - bluez: 32
> - gnome-settings-daemon: 1024
> If this is the case, we will consider that we have to use the original size
> (RFKILL_EVENT_SIZE_V1) and old applications will be able to work as ever.
> For other values, we will follow the new behavior with extended events.

Now, however, applications that do use 32 or 1024 as the buffer size,
the latter of which, btw, is just the default glib size, not related to
g-s-d, will never be able to get the new data. They don't really need it
for now, but if we add anything else and keep this workaround,
applications will have to work around the kernel *again* and change
their read buffer size - which in the case of g-s-d, as I said above,
isn't even a conscious choice but comes from glib.

> No other applications have been identified that behave in this way, so
> reserved event sizes are defined.

I'm going to assume that you're doing this for RHEL, in which case I'm
not sure this even addresses your requirements - if there's an
application with read size 64 that you don't know about, it would still
possibly be broken?

I tend to think if you need a stable guarantee here you should probably
just revert the kernel patch instead.


I know there's no good choice here. This thing was intended to be
forward and backward compatible, but implementations got it wrong. We
therefore had a regression, but it was noticed fairly late after being
introduced, and so it was a bit difficult to revert. Also, there was
really no good other choice - perhaps we could have added /dev/rfkill2,
but ...

Ultimately, all the userspace folks basically said "oh yeah we got this
wrong" and fixed it pretty much immediately.

johannes