From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752020AbdJYOQz (ORCPT ); Wed, 25 Oct 2017 10:16:55 -0400 Received: from mail-io0-f194.google.com ([209.85.223.194]:56116 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751863AbdJYOQv (ORCPT ); Wed, 25 Oct 2017 10:16:51 -0400 X-Google-Smtp-Source: ABhQp+TLtQ2MzStpJgyFdRbzif7LGDQ0x2Nm7uSX1X90KNB8tVmc0XBpIc6V+32pFNb6+UiqoeczCJOt4Ndy2uYm2FU= MIME-Version: 1.0 In-Reply-To: <20171024100859.GA69126@beast> References: <20171024100859.GA69126@beast> From: Kees Cook Date: Wed, 25 Oct 2017 16:16:49 +0200 X-Google-Sender-Auth: kK-t-nGeHGNI4XzWzI1hZSOtYpc Message-ID: Subject: Re: [PATCH] usb: usbatm: Convert timers to use timer_setup() To: Greg Kroah-Hartman Cc: Duncan Sands , Allen Pais , "Gustavo A. R. Silva" , accessrunner-general@lists.sourceforge.net, linux-usb@vger.kernel.org, LKML Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 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 > Cc: Duncan Sands > Cc: Allen Pais > Cc: "Gustavo A. R. Silva" > Cc: accessrunner-general@lists.sourceforge.net > Cc: linux-usb@vger.kernel.org > Signed-off-by: Kees Cook > --- > 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