Re: [PATCH RESEND] bluetooth:hicp:Fix error handling in the function hidp_connection_del

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

 



Hi

On Tue, Dec 29, 2015 at 9:55 PM, Nicholas Krause <xerofoify@xxxxxxxxx> wrote:
> This fixes error handling in the function hidp_connection_del to
> properly check if the internal call to hidp_send_ctrl_message has
> failed by returning a error code and if so immediately return this
> error code to the caller of hidp_connection_del to signal that a
> failed call has occurred that needs to be handled by this function's
> caller.
>
> Signed-off-by: Nicholas Krause <xerofoify@xxxxxxxxx>
> ---
>  net/bluetooth/hidp/core.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index f1a117f..13aa6d0 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -1360,6 +1360,7 @@ int hidp_connection_del(struct hidp_conndel_req *req)
>  {
>         u32 valid_flags = BIT(HIDP_VIRTUAL_CABLE_UNPLUG);
>         struct hidp_session *session;
> +       int err;
>
>         if (req->flags & ~valid_flags)
>                 return -EINVAL;
> @@ -1368,14 +1369,16 @@ int hidp_connection_del(struct hidp_conndel_req *req)
>         if (!session)
>                 return -ENOENT;
>
> -       if (req->flags & BIT(HIDP_VIRTUAL_CABLE_UNPLUG))
> -               hidp_send_ctrl_message(session,
> -                                      HIDP_TRANS_HID_CONTROL |
> -                                        HIDP_CTRL_VIRTUAL_CABLE_UNPLUG,
> -                                      NULL, 0);
> -       else
> +       if (req->flags & BIT(HIDP_VIRTUAL_CABLE_UNPLUG)) {
> +               err = hidp_send_ctrl_message(session,
> +                                            HIDP_TRANS_HID_CONTROL |
> +                                            HIDP_CTRL_VIRTUAL_CABLE_UNPLUG,
> +                                            NULL, 0);
> +               if (err)
> +                       return err;

This leaks a reference to 'session'.

> +       } else {
>                 l2cap_unregister_user(session->conn, &session->user);
> -
> +       }
>         hidp_session_put(session);

I'm not very convinced that this is a good idea. Transmission is
handled async, hence all you catch here is OOM. Nothing interesting
can be returned, and there's nothing sane the caller can do to handle
the error. I'd keep the code as is.

Thanks
David

>
>         return 0;
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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