| Message ID | 20220315102641.233703-1-jtornosm@redhat.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | rfkill: keep rfkill event compatibility with old userspace applications | expand |
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
diff --git a/include/uapi/linux/rfkill.h b/include/uapi/linux/rfkill.h index 9b77cfc42efa..821e304a1d8e 100644 --- a/include/uapi/linux/rfkill.h +++ b/include/uapi/linux/rfkill.h @@ -168,8 +168,14 @@ struct rfkill_event_ext { * older kernel; * 3. treat reads that are as long as requested as acceptable, not * checking against RFKILL_EVENT_SIZE_V1 or such. + * 4. in order to avoid compatibilities issues with older application + * versions specifying unusual event size requests, those unusual + * request event sizes will be considered reserved. If requested size + * is reserved, the event size will be RFKILL_EVENT_SIZE_V1. */ #define RFKILL_EVENT_SIZE_V1 sizeof(struct rfkill_event) +#define RESERVED_RFKILL_EVENT_SIZE_1 32 +#define RESERVED_RFKILL_EVENT_SIZE_2 1024 /* ioctl for turning off rfkill-input (if present) */ #define RFKILL_IOC_MAGIC 'R' diff --git a/net/rfkill/core.c b/net/rfkill/core.c index b73a741a7923..494335d4f5f7 100644 --- a/net/rfkill/core.c +++ b/net/rfkill/core.c @@ -1231,7 +1231,13 @@ static ssize_t rfkill_fop_read(struct file *file, char __user *buf, ev = list_first_entry(&data->events, struct rfkill_int_event, list); - sz = min_t(unsigned long, sizeof(ev->ev), count); + BUILD_BUG_ON(sizeof(ev->ev) == RESERVED_RFKILL_EVENT_SIZE_1 || + sizeof(ev->ev) == RESERVED_RFKILL_EVENT_SIZE_2); + if (count == RESERVED_RFKILL_EVENT_SIZE_1 || + count == RESERVED_RFKILL_EVENT_SIZE_2) + sz = RFKILL_EVENT_SIZE_V1; + else + sz = min_t(unsigned long, sizeof(ev->ev), count); ret = sz; if (copy_to_user(buf, &ev->ev, sz)) ret = -EFAULT;
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. The reason is the new extended rfkill event format that is used by kernel, if requested size is big enough. Detailed operation of commented bluez versions, by means of strace output: read(11, "\0\0\0\0\2\2\1\0\0", 32) = 9 That is, as the new rfkill event size is 9, it will be rejected by commented bluez versions (expected size 8). 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. No other applications have been identified that behave in this way, so reserved event sizes are defined. Fixes: 71826654ce40 ("rfkill: revert back to old userspace API by default") Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com> --- include/uapi/linux/rfkill.h | 6 ++++++ net/rfkill/core.c | 8 +++++++- 2 files changed, 13 insertions(+), 1 deletion(-)