Hi Tetsuo,
On Sat, Jul 24, 2021 at 7:03 AM Tetsuo Handa
<penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote:
>
> hci_sock.c has many
> hci_send_to_channel(HCI_CHANNEL_MONITOR, skb, HCI_SOCK_TRUSTED, NULL);
> calls. Use helper functions and replace skb with sk.
>
> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> ---
> net/bluetooth/hci_sock.c | 96 ++++++++++++++++------------------------
> 1 file changed, 37 insertions(+), 59 deletions(-)
>
> diff --git a/net/bluetooth/hci_sock.c b/net/bluetooth/hci_sock.c
> index 786a06a232fd..fc2336855dab 100644
> --- a/net/bluetooth/hci_sock.c
> +++ b/net/bluetooth/hci_sock.c
> @@ -295,6 +295,11 @@ void hci_send_to_channel(unsigned short channel, struct sk_buff *skb,
> read_unlock(&hci_sk_list.lock);
> }
>
> +static void __hci_send_to_monitor(struct sk_buff *skb)
> +{
We can probably have the checks of NULL skb added directly here and
perhaps kfree_skb as well since it seems it is always a copy that is
sent here, the hci_send_to_monitor don't have __ prefix so I wonder
why you have chosen to use it?
> + hci_send_to_channel(HCI_CHANNEL_MONITOR, skb, HCI_SOCK_TRUSTED, NULL);
> +}
> +
> /* Send frame to monitor socket */
> void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
> {
> @@ -350,8 +355,7 @@ void hci_send_to_monitor(struct hci_dev *hdev, struct sk_buff *skb)
> hdr->index = cpu_to_le16(hdev->id);
> hdr->len = cpu_to_le16(skb->len);
>
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb_copy,
> - HCI_SOCK_TRUSTED, NULL);
> + __hci_send_to_monitor(skb_copy);
> kfree_skb(skb_copy);
> }
>
> @@ -545,6 +549,16 @@ static struct sk_buff *create_monitor_ctrl_open(struct sock *sk)
> return skb;
> }
>
> +static void hci_monitor_ctrl_open(struct sock *sk)
> +{
> + struct sk_buff *skb = create_monitor_ctrl_open(sk);
> +
> + if (skb) {
> + __hci_send_to_monitor(skb);
> + kfree_skb(skb);
> + }
> +}
> +
> static struct sk_buff *create_monitor_ctrl_close(struct sock *sk)
> {
> struct hci_mon_hdr *hdr;
> @@ -583,6 +597,16 @@ static struct sk_buff *create_monitor_ctrl_close(struct sock *sk)
> return skb;
> }
>
> +static void hci_monitor_ctrl_close(struct sock *sk)
> +{
> + struct sk_buff *skb = create_monitor_ctrl_close(sk);
> +
> + if (skb) {
> + __hci_send_to_monitor(skb);
> + kfree_skb(skb);
> + }
> +}
> +
> static struct sk_buff *create_monitor_ctrl_command(struct sock *sk, u16 index,
> u16 opcode, u16 len,
> const void *buf)
> @@ -741,8 +765,7 @@ void hci_sock_dev_event(struct hci_dev *hdev, int event)
> /* Send event to monitor */
> skb = create_monitor_event(hdev, event);
> if (skb) {
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> - HCI_SOCK_TRUSTED, NULL);
> + __hci_send_to_monitor(skb);
> kfree_skb(skb);
> }
> }
> @@ -859,7 +882,6 @@ static int hci_sock_release(struct socket *sock)
> {
> struct sock *sk = sock->sk;
> struct hci_dev *hdev;
> - struct sk_buff *skb;
>
> BT_DBG("sock %p sk %p", sock, sk);
>
> @@ -876,12 +898,7 @@ static int hci_sock_release(struct socket *sock)
> case HCI_CHANNEL_USER:
> case HCI_CHANNEL_CONTROL:
> /* Send event to monitor */
> - skb = create_monitor_ctrl_close(sk);
> - if (skb) {
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> - HCI_SOCK_TRUSTED, NULL);
> - kfree_skb(skb);
> - }
> + hci_monitor_ctrl_close(sk);
>
> hci_sock_free_cookie(sk);
> break;
> @@ -1021,18 +1038,11 @@ static int hci_sock_ioctl(struct socket *sock, unsigned int cmd,
> * of a given socket.
> */
> if (hci_sock_gen_cookie(sk)) {
> - struct sk_buff *skb;
> -
> if (capable(CAP_NET_ADMIN))
> hci_sock_set_flag(sk, HCI_SOCK_TRUSTED);
>
> /* Send event to monitor */
> - skb = create_monitor_ctrl_open(sk);
> - if (skb) {
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> - HCI_SOCK_TRUSTED, NULL);
> - kfree_skb(skb);
> - }
> + hci_monitor_ctrl_open(sk);
> }
>
> release_sock(sk);
> @@ -1114,7 +1124,6 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
> struct sockaddr_hci haddr;
> struct sock *sk = sock->sk;
> struct hci_dev *hdev = NULL;
> - struct sk_buff *skb;
> int len, err = 0;
>
> BT_DBG("sock %p sk %p", sock, sk);
> @@ -1162,12 +1171,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
> * notification. Send a close notification first to
> * allow the state transition to bounded.
> */
> - skb = create_monitor_ctrl_close(sk);
> - if (skb) {
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> - HCI_SOCK_TRUSTED, NULL);
> - kfree_skb(skb);
> - }
> + hci_monitor_ctrl_close(sk);
> }
>
> if (capable(CAP_NET_ADMIN))
> @@ -1176,12 +1180,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
> hci_pi(sk)->hdev = hdev;
>
> /* Send event to monitor */
> - skb = create_monitor_ctrl_open(sk);
> - if (skb) {
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> - HCI_SOCK_TRUSTED, NULL);
> - kfree_skb(skb);
> - }
> + hci_monitor_ctrl_open(sk);
> break;
>
> case HCI_CHANNEL_USER:
> @@ -1251,12 +1250,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
> * a user channel socket. For a clean transition, send
> * the close notification first.
> */
> - skb = create_monitor_ctrl_close(sk);
> - if (skb) {
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> - HCI_SOCK_TRUSTED, NULL);
> - kfree_skb(skb);
> - }
> + hci_monitor_ctrl_close(sk);
> }
>
> /* The user channel is restricted to CAP_NET_ADMIN
> @@ -1267,12 +1261,7 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
> hci_pi(sk)->hdev = hdev;
>
> /* Send event to monitor */
> - skb = create_monitor_ctrl_open(sk);
> - if (skb) {
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> - HCI_SOCK_TRUSTED, NULL);
> - kfree_skb(skb);
> - }
> + hci_monitor_ctrl_open(sk);
>
> atomic_inc(&hdev->promisc);
> break;
> @@ -1359,21 +1348,11 @@ static int hci_sock_bind(struct socket *sock, struct sockaddr *addr,
> * allow for a clean transition, send the
> * close notification first.
> */
> - skb = create_monitor_ctrl_close(sk);
> - if (skb) {
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> - HCI_SOCK_TRUSTED, NULL);
> - kfree_skb(skb);
> - }
> + hci_monitor_ctrl_close(sk);
> }
>
> /* Send event to monitor */
> - skb = create_monitor_ctrl_open(sk);
> - if (skb) {
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> - HCI_SOCK_TRUSTED, NULL);
> - kfree_skb(skb);
> - }
> + hci_monitor_ctrl_open(sk);
>
> hci_sock_set_flag(sk, HCI_MGMT_INDEX_EVENTS);
> hci_sock_set_flag(sk, HCI_MGMT_UNCONF_INDEX_EVENTS);
> @@ -1559,8 +1538,7 @@ static int hci_mgmt_cmd(struct hci_mgmt_chan *chan, struct sock *sk,
> skb = create_monitor_ctrl_command(sk, index, opcode, len,
> buf + sizeof(*hdr));
> if (skb) {
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb,
> - HCI_SOCK_TRUSTED, NULL);
> + __hci_send_to_monitor(skb);
> kfree_skb(skb);
> }
> }
> @@ -1715,7 +1693,7 @@ static int hci_logging_frame(struct sock *sk, struct msghdr *msg, int len)
>
> hdr->opcode = cpu_to_le16(HCI_MON_USER_LOGGING);
>
> - hci_send_to_channel(HCI_CHANNEL_MONITOR, skb, HCI_SOCK_TRUSTED, NULL);
> + __hci_send_to_monitor(skb);
> err = len;
>
> if (hdev)
> --
> 2.18.4
>
--
Luiz Augusto von Dentz
