Re: [PATCH] usb: usbatm: Convert timers to use timer_setup()
From: Kees Cook
Date: Wed Oct 25 2017 - 10:16:58 EST
This also uses timer_setup_on_stack() (only in -next). If it's okay,
I'll carry it in the timers tree.
Thanks!
-Kees
On Tue, Oct 24, 2017 at 12:08 PM, Kees Cook <keescook@xxxxxxxxxxxx> wrote:
> In preparation for unconditionally passing the struct timer_list pointer to
> all timer callbacks, switch to using the new timer_setup() and from_timer()
> to pass the timer pointer explicitly. Additionally corrects and on-stack
> timer usage.
>
> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> Cc: Duncan Sands <duncan.sands@xxxxxxx>
> Cc: Allen Pais <allen.lkml@xxxxxxxxx>
> Cc: "Gustavo A. R. Silva" <garsilva@xxxxxxxxxxxxxx>
> Cc: accessrunner-general@xxxxxxxxxxxxxxxxxxxxx
> Cc: linux-usb@xxxxxxxxxxxxxxx
> Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
> ---
> drivers/usb/atm/cxacru.c | 23 ++++++++++++++++-------
> drivers/usb/atm/speedtch.c | 16 ++++++++--------
> drivers/usb/atm/usbatm.c | 10 +++++-----
> 3 files changed, 29 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/usb/atm/cxacru.c b/drivers/usb/atm/cxacru.c
> index 600a670b4feb..53c00d035e31 100644
> --- a/drivers/usb/atm/cxacru.c
> +++ b/drivers/usb/atm/cxacru.c
> @@ -560,21 +560,30 @@ static void cxacru_blocking_completion(struct urb *urb)
> complete(urb->context);
> }
>
> -static void cxacru_timeout_kill(unsigned long data)
> +struct cxacru_timer {
> + struct timer_list timer;
> + struct urb *urb;
> +};
> +
> +static void cxacru_timeout_kill(struct timer_list *t)
> {
> - usb_unlink_urb((struct urb *) data);
> + struct cxacru_timer *timer = from_timer(timer, t, timer);
> +
> + usb_unlink_urb(timer->urb);
> }
>
> static int cxacru_start_wait_urb(struct urb *urb, struct completion *done,
> int *actual_length)
> {
> - struct timer_list timer;
> + struct cxacru_timer timer = {
> + .urb = urb,
> + };
>
> - setup_timer(&timer, cxacru_timeout_kill, (unsigned long)urb);
> - timer.expires = jiffies + msecs_to_jiffies(CMD_TIMEOUT);
> - add_timer(&timer);
> + timer_setup_on_stack(&timer.timer, cxacru_timeout_kill, 0);
> + mod_timer(&timer.timer, jiffies + msecs_to_jiffies(CMD_TIMEOUT));
> wait_for_completion(done);
> - del_timer_sync(&timer);
> + del_timer_sync(&timer.timer);
> + destroy_timer_on_stack(&timer.timer);
>
> if (actual_length)
> *actual_length = urb->actual_length;
> diff --git a/drivers/usb/atm/speedtch.c b/drivers/usb/atm/speedtch.c
> index 091db9b281f5..b7fb4beed2b8 100644
> --- a/drivers/usb/atm/speedtch.c
> +++ b/drivers/usb/atm/speedtch.c
> @@ -571,9 +571,10 @@ static void speedtch_check_status(struct work_struct *work)
> }
> }
>
> -static void speedtch_status_poll(unsigned long data)
> +static void speedtch_status_poll(struct timer_list *t)
> {
> - struct speedtch_instance_data *instance = (void *)data;
> + struct speedtch_instance_data *instance =
> + from_timer(instance, t, status_check_timer);
>
> schedule_work(&instance->status_check_work);
>
> @@ -584,9 +585,10 @@ static void speedtch_status_poll(unsigned long data)
> atm_warn(instance->usbatm, "Too many failures - disabling line status polling\n");
> }
>
> -static void speedtch_resubmit_int(unsigned long data)
> +static void speedtch_resubmit_int(struct timer_list *t)
> {
> - struct speedtch_instance_data *instance = (void *)data;
> + struct speedtch_instance_data *instance = from_timer(instance, t,
> + resubmit_timer);
> struct urb *int_urb = instance->int_urb;
> int ret;
>
> @@ -874,13 +876,11 @@ static int speedtch_bind(struct usbatm_data *usbatm,
> usbatm->flags |= (use_isoc ? UDSL_USE_ISOC : 0);
>
> INIT_WORK(&instance->status_check_work, speedtch_check_status);
> - setup_timer(&instance->status_check_timer, speedtch_status_poll,
> - (unsigned long)instance);
> + timer_setup(&instance->status_check_timer, speedtch_status_poll, 0);
> instance->last_status = 0xff;
> instance->poll_delay = MIN_POLL_DELAY;
>
> - setup_timer(&instance->resubmit_timer, speedtch_resubmit_int,
> - (unsigned long)instance);
> + timer_setup(&instance->resubmit_timer, speedtch_resubmit_int, 0);
>
> instance->int_urb = usb_alloc_urb(0, GFP_KERNEL);
>
> diff --git a/drivers/usb/atm/usbatm.c b/drivers/usb/atm/usbatm.c
> index 8607af758bbd..31a4e4824f23 100644
> --- a/drivers/usb/atm/usbatm.c
> +++ b/drivers/usb/atm/usbatm.c
> @@ -1003,18 +1003,18 @@ static int usbatm_heavy_init(struct usbatm_data *instance)
> return 0;
> }
>
> -static void usbatm_tasklet_schedule(unsigned long data)
> +static void usbatm_tasklet_schedule(struct timer_list *t)
> {
> - tasklet_schedule((struct tasklet_struct *) data);
> + struct usbatm_channel *channel = from_timer(channel, t, delay);
> +
> + tasklet_schedule(&channel->tasklet);
> }
>
> static void usbatm_init_channel(struct usbatm_channel *channel)
> {
> spin_lock_init(&channel->lock);
> INIT_LIST_HEAD(&channel->list);
> - channel->delay.function = usbatm_tasklet_schedule;
> - channel->delay.data = (unsigned long) &channel->tasklet;
> - init_timer(&channel->delay);
> + timer_setup(&channel->delay, usbatm_tasklet_schedule, 0);
> }
>
> int usbatm_usb_probe(struct usb_interface *intf, const struct usb_device_id *id,
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security
--
Kees Cook
Pixel Security