| Message ID | 20220315124811.237037-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 |
Hi, It'd be nicer if you were to actually reply in the thread. Now I even have to copy/paste because you put everything into the signature :( Don't make it so hard to interact! > > 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. > > > I really wanted to say "because they're checking the received size", that is, > because older bluez is expecting 8, and as bluez bluez is receiving 9, it was > rejecting the event, and therefore it is broken. Well, ok, then "because they're incorrectly checking the received size". > I tested g-s-d without their fixes and it keeps on working fine because > although the message was buffered, it took 8 byte event size and when it read > immediately again, it got another message with 1 byte that was rejected (8 > bytes expected size) and this way exit from the loop. Not sure that's how it works? Before the fixes there in g-s-d, it would have accumulated the extra bytes until they were 8 bytes and a new message was formed, or something like that, no? Anyway, you can still argue we broke it by changing the size. > Yes I agree with you, it would be enough and easy to always send the 8 > bytes > events from the kernel as before (at least for RHEL). > But, I am just trying to do it in a more compatible way, because as > you said > we do not know the behavior of all applications using the rfkill. But you're doing it *less* compatible. If you want to be perfectly compatible, you have to revert the entire new event size. You're just hacking around the edges to make the two broken cases you found work. I really don't think that's a good idea. At the time, I was tempted to add an ioctl, so you'd have to do ioctl(fd, RFKILL_I_READ_THE_DOCS_ABOUT_STRUCT_SIZES, RFKILL_AND_I_PROMISE_TO_READ_NON_STREAMING); to get extended messages out at all ... Maybe should've done that. Right now, nobody really has to care about the extra field at all anyway. But honestly, compared to fixing two or three userspace applications (bluez and g-s-d, systemd just had an API not ABI issue that we could work around) seemed simpler? 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(-)