Re: [PATCH] rfkill: Fix reading from rfkill socket

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2021-05-03 at 15:12 +0200, Benjamin Berg wrote:
> From: Benjamin Berg <bberg@xxxxxxxxxx>
> 
> The kernel will always send exactly one event, but the size of the
> passed struct will depend on the length of the submitted read() and the
> kernel version. i.e. the interface can be extended and we need to
> expect
> for a read to be longer than expected if we ask for it.
> 
> Fix this by only requesting the needed length and explicitly check the
> length against the V1 version of the structure to make the code a bit
> more future proof in case the internal copy of the struct is updated to
> contain new fields.

This fixes a bug in GNOME where to enable Bluetooth, we removed a soft
rfkill block on the Bluetooth interface.

Without this, the bluetooth rfkill gets unblocked, but bluetoothd
doesn't see it as unblocked so never powers it on, causing the UI to
appear broken, as we expect Bluetooth devices to be either blocked
through rfkill, or powered on.

The equivalent gnome-settings-daemon fix (which deals with rfkill) was
reviewed by Hans de Goede:
https://gitlab.gnome.org/GNOME/gnome-settings-daemon/-/merge_requests/234

Benjamin, it might be worth resending this with a better commit message
explaining exactly what it fixes and referencing the gnome-bluetooth
bug:
https://gitlab.gnome.org/GNOME/gnome-bluetooth/-/issues/38

Cheers

> ---
>  src/rfkill.c | 24 +++++++++++-------------
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/src/rfkill.c b/src/rfkill.c
> index ec9fcdfdd..2099c5ac5 100644
> --- a/src/rfkill.c
> +++ b/src/rfkill.c
> @@ -53,12 +53,12 @@ struct rfkill_event {
>         uint8_t  soft;
>         uint8_t  hard;
>  };
> +#define RFKILL_EVENT_SIZE_V1    8
>  
>  static gboolean rfkill_event(GIOChannel *chan,
>                                 GIOCondition cond, gpointer data)
>  {
> -       unsigned char buf[32];
> -       struct rfkill_event *event = (void *) buf;
> +       struct rfkill_event event = { 0 };
>         struct btd_adapter *adapter;
>         char sysname[PATH_MAX];
>         ssize_t len;
> @@ -69,34 +69,32 @@ static gboolean rfkill_event(GIOChannel *chan,
>  
>         fd = g_io_channel_unix_get_fd(chan);
>  
> -       memset(buf, 0, sizeof(buf));
> -
> -       len = read(fd, buf, sizeof(buf));
> +       len = read(fd, &event, sizeof(event));
>         if (len < 0) {
>                 if (errno == EAGAIN)
>                         return TRUE;
>                 return FALSE;
>         }
>  
> -       if (len != sizeof(struct rfkill_event))
> +       if (len < RFKILL_EVENT_SIZE_V1)
>                 return TRUE;
>  
>         DBG("RFKILL event idx %u type %u op %u soft %u hard %u",
> -                                       event->idx, event->type, event-
> >op,
> -                                               event->soft, event-
> >hard);
> +                                       event.idx, event.type,
> event.op,
> +                                               event.soft,
> event.hard);
>  
> -       if (event->soft || event->hard)
> +       if (event.soft || event.hard)
>                 return TRUE;
>  
> -       if (event->op != RFKILL_OP_CHANGE)
> +       if (event.op != RFKILL_OP_CHANGE)
>                 return TRUE;
>  
> -       if (event->type != RFKILL_TYPE_BLUETOOTH &&
> -                                       event->type != RFKILL_TYPE_ALL)
> +       if (event.type != RFKILL_TYPE_BLUETOOTH &&
> +                                       event.type != RFKILL_TYPE_ALL)
>                 return TRUE;
>  
>         snprintf(sysname, sizeof(sysname) - 1,
> -                       "/sys/class/rfkill/rfkill%u/name", event->idx);
> +                       "/sys/class/rfkill/rfkill%u/name", event.idx);
>  
>         fd = open(sysname, O_RDONLY);
>         if (fd < 0)





[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux