| Message ID | 1508200182-104605-33-git-send-email-keescook@chromium.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | networking: Convert timers to use timer_setup() | expand |
On Mon, 2017-10-16 at 17:29 -0700, 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. Acked-by: Paul Bolle <pebolle@tiscali.nl> For the record: this patch made me nervous but survived the rigorous testing I threw at it. (Ie, dialing up using bas_gigaset and downloading almost 20 MB in just over an hour. Whoot! That's more than good enough to ack this patch.) There was some cleanup I had in mind to make this patch more straightforward. But that can wait until someone finds a way to hit an issue with this patch. We'll see. Thanks, Paul Bolle
On Thu, 2017-10-19 at 23:03 +0200, Paul Bolle wrote: > On Mon, 2017-10-16 at 17:29 -0700, 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. > > Acked-by: Paul Bolle <pebolle@tiscali.nl> I have to take this back, sorry! > For the record: this patch made me nervous but survived the rigorous testing I > threw at it. (Ie, dialing up using bas_gigaset and downloading almost 20 MB in > just over an hour. Whoot! That's more than good enough to ack this patch.) > > There was some cleanup I had in mind to make this patch more straightforward. > But that can wait until someone finds a way to hit an issue with this patch. > We'll see. That someone turns out to be me, doing "modprobe -r bas_gigaset": <1>[30143.538135] BUG: unable to handle kernel NULL pointer dereference at 000001e9 <1>[30143.538154] IP: mutex_lock+0x19/0x30 <6>[30143.538157] *pde = 00000000 <5>[30143.538162] Oops: 0002 [#1] SMP <5>[30143.538165] Modules linked in: bas_gigaset(OE-) gigaset vfat fat uas usb_storage ppp_deflate bsd_comp ppp_synctty ppp_generic slhc capi kernelcapi fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c iptable_mangle iptable_raw iptable_security ebtable_filter ebtables ip6table_filter ip6_tables sunrpc snd_intel8x0 snd_ac97_codec gpio_ich ac97_bus ppdev iTCO_wdt iTCO_vendor_support lpc_ich snd_seq snd_seq_device snd_pcm pcspkr i2c_i801 thinkpad_acpi <5>[30143.538228] snd_timer snd irda(C) soundcore parport_pc rfkill parport acpi_cpufreq i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops sdhci_pci drm tg3 sdhci mmc_core ata_generic serio_raw yenta_socket ptp pata_acpi pps_core video [last unloaded: gigaset] <0>[30143.538257] CPU: 0 PID: 22085 Comm: modprobe Tainted: G C OE 4.14.0-0.rc4.1.local0.fc26.i686 #1 <0>[30143.538260] Hardware name: IBM 2525FAG/2525FAG, BIOS 74ET64WW (2.09 ) 12/14/2006 <0>[30143.538263] task: f6671100 task.stack: c77c6000 <5>[30143.538267] EIP: mutex_lock+0x19/0x30 <5>[30143.538269] EFLAGS: 00010246 CPU: 0 <5>[30143.538272] EAX: 00000000 EBX: 000001e9 ECX: 00000001 EDX: f6671100 <5>[30143.538275] ESI: 000001e9 EDI: 011c8f98 EBP: c77c7ef4 ESP: c77c7ef0 <5>[30143.538278] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 <5>[30143.538281] CR0: 80050033 CR2: 000001e9 CR3: 16d2d000 CR4: 000006d0 <0>[30143.538284] Call Trace: <0>[30143.538300] gigaset_shutdown+0x28/0x130 [gigaset] <0>[30143.538307] ? find_module_all+0x62/0x80 <0>[30143.538314] bas_gigaset_exit+0x31/0x1077 [bas_gigaset] <0>[30143.538319] SyS_delete_module+0x19c/0x240 <0>[30143.538325] ? ____fput+0xd/0x10 <0>[30143.538330] do_fast_syscall_32+0x71/0x1a0 <0>[30143.538335] entry_SYSENTER_32+0x4e/0x7c <5>[30143.538337] EIP: 0xb7fb5cd9 <5>[30143.538339] EFLAGS: 00000206 CPU: 0 <5>[30143.538342] EAX: ffffffda EBX: 011c8fd4 ECX: 00000800 EDX: 011c8fd4 <5>[30143.538345] ESI: 011c8f98 EDI: 011c8f98 EBP: 011c8fd4 ESP: bf8278f8 <5>[30143.538348] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b <0>[30143.538351] Code: 8e fb ff ff 5d c3 8d b6 00 00 00 00 8d bf 00 00 00 00 3e 8d 74 26 00 55 89 e5 53 89 c3 e8 60 e7 ff ff 64 8b 15 40 c9 82 c7 31 c0 <3e> 0f b1 13 85 c0 74 07 89 d8 e8 b8 ff ff ff 5b 5d c3 90 8d 74 <0>[30143.538399] EIP: mutex_lock+0x19/0x30 SS:ESP: 0068:c77c7ef0 <5>[30143.538402] CR2: 00000000000001e9 <4>[30143.538406] ---[ end trace 3e60af64adfe7e14 ]--- I'll have to ask for some patience so I can find out what's going on. (Very likely: using urb->context beyond it's lifetime.) Expect something by early next week. Is that OK with you? Sorry for the noise, Paul Bolle
On Thu, 19 Oct 2017, Paul Bolle wrote: > On Thu, 2017-10-19 at 23:03 +0200, Paul Bolle wrote: > > On Mon, 2017-10-16 at 17:29 -0700, 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. > > > > Acked-by: Paul Bolle <pebolle@tiscali.nl> > > I have to take this back, sorry! > > > For the record: this patch made me nervous but survived the rigorous testing I > > threw at it. (Ie, dialing up using bas_gigaset and downloading almost 20 MB in > > just over an hour. Whoot! That's more than good enough to ack this patch.) > > > > There was some cleanup I had in mind to make this patch more straightforward. > > But that can wait until someone finds a way to hit an issue with this patch. > > We'll see. > > That someone turns out to be me, doing "modprobe -r bas_gigaset": > > <1>[30143.538135] BUG: unable to handle kernel NULL pointer dereference at 000001e9 > <1>[30143.538154] IP: mutex_lock+0x19/0x30 > <0>[30143.538300] gigaset_shutdown+0x28/0x130 [gigaset] > <0>[30143.538307] ? find_module_all+0x62/0x80 > <0>[30143.538314] bas_gigaset_exit+0x31/0x1077 [bas_gigaset] bas_gigaset_exit() { for (i = 0; i < driver->minors; i++) { if (gigaset_shutdown(driver->cs + i) < 0) gigaset_shutdown(cs) { mutex_lock(&cs->mutex); <-------- Explodes here So driver->cs + i is invalid. No idea how that might be related to that timer conversion patch, but .... Thanks, tglx
On Thu, Oct 19, 2017 at 2:20 PM, Paul Bolle <pebolle@tiscali.nl> wrote: > On Thu, 2017-10-19 at 23:03 +0200, Paul Bolle wrote: >> On Mon, 2017-10-16 at 17:29 -0700, 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. >> >> Acked-by: Paul Bolle <pebolle@tiscali.nl> > > I have to take this back, sorry! > >> For the record: this patch made me nervous but survived the rigorous testing I >> threw at it. (Ie, dialing up using bas_gigaset and downloading almost 20 MB in >> just over an hour. Whoot! That's more than good enough to ack this patch.) >> >> There was some cleanup I had in mind to make this patch more straightforward. >> But that can wait until someone finds a way to hit an issue with this patch. >> We'll see. > > That someone turns out to be me, doing "modprobe -r bas_gigaset": > > <1>[30143.538135] BUG: unable to handle kernel NULL pointer dereference at 000001e9 > <1>[30143.538154] IP: mutex_lock+0x19/0x30 > <6>[30143.538157] *pde = 00000000 > <5>[30143.538162] Oops: 0002 [#1] SMP > <5>[30143.538165] Modules linked in: bas_gigaset(OE-) gigaset vfat fat uas usb_storage ppp_deflate bsd_comp ppp_synctty ppp_generic slhc capi kernelcapi fuse xt_CHECKSUM ipt_MASQUERADE nf_nat_masquerade_ipv4 tun nf_conntrack_netbios_ns nf_conntrack_broadcast xt_CT ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack libcrc32c iptable_mangle iptable_raw iptable_security ebtable_filter ebtables ip6table_filter ip6_tables sunrpc snd_intel8x0 snd_ac97_codec gpio_ich ac97_bus ppdev iTCO_wdt iTCO_vendor_support lpc_ich snd_seq snd_seq_device snd_pcm pcspkr i2c_i801 thinkpad_acpi > <5>[30143.538228] snd_timer snd irda(C) soundcore parport_pc rfkill parport acpi_cpufreq i915 i2c_algo_bit drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops sdhci_pci drm tg3 sdhci mmc_core ata_generic serio_raw yenta_socket ptp pata_acpi pps_core video [last unloaded: gigaset] > <0>[30143.538257] CPU: 0 PID: 22085 Comm: modprobe Tainted: G C OE 4.14.0-0.rc4.1.local0.fc26.i686 #1 > <0>[30143.538260] Hardware name: IBM 2525FAG/2525FAG, BIOS 74ET64WW (2.09 ) 12/14/2006 > <0>[30143.538263] task: f6671100 task.stack: c77c6000 > <5>[30143.538267] EIP: mutex_lock+0x19/0x30 > <5>[30143.538269] EFLAGS: 00010246 CPU: 0 > <5>[30143.538272] EAX: 00000000 EBX: 000001e9 ECX: 00000001 EDX: f6671100 > <5>[30143.538275] ESI: 000001e9 EDI: 011c8f98 EBP: c77c7ef4 ESP: c77c7ef0 > <5>[30143.538278] DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 > <5>[30143.538281] CR0: 80050033 CR2: 000001e9 CR3: 16d2d000 CR4: 000006d0 > <0>[30143.538284] Call Trace: > <0>[30143.538300] gigaset_shutdown+0x28/0x130 [gigaset] > <0>[30143.538307] ? find_module_all+0x62/0x80 > <0>[30143.538314] bas_gigaset_exit+0x31/0x1077 [bas_gigaset] > <0>[30143.538319] SyS_delete_module+0x19c/0x240 > <0>[30143.538325] ? ____fput+0xd/0x10 > <0>[30143.538330] do_fast_syscall_32+0x71/0x1a0 > <0>[30143.538335] entry_SYSENTER_32+0x4e/0x7c > <5>[30143.538337] EIP: 0xb7fb5cd9 > <5>[30143.538339] EFLAGS: 00000206 CPU: 0 > <5>[30143.538342] EAX: ffffffda EBX: 011c8fd4 ECX: 00000800 EDX: 011c8fd4 > <5>[30143.538345] ESI: 011c8f98 EDI: 011c8f98 EBP: 011c8fd4 ESP: bf8278f8 > <5>[30143.538348] DS: 007b ES: 007b FS: 0000 GS: 0033 SS: 007b > <0>[30143.538351] Code: 8e fb ff ff 5d c3 8d b6 00 00 00 00 8d bf 00 00 00 00 3e 8d 74 26 00 55 89 e5 53 89 c3 e8 60 e7 ff ff 64 8b 15 40 c9 82 c7 31 c0 <3e> 0f b1 13 85 c0 74 07 89 d8 e8 b8 ff ff ff 5b 5d c3 90 8d 74 > <0>[30143.538399] EIP: mutex_lock+0x19/0x30 SS:ESP: 0068:c77c7ef0 > <5>[30143.538402] CR2: 00000000000001e9 > <4>[30143.538406] ---[ end trace 3e60af64adfe7e14 ]--- > > I'll have to ask for some patience so I can find out what's going on. (Very > likely: using urb->context beyond it's lifetime.) Expect something by early Eek, thanks for finding this. > next week. Is that OK with you? Totally fine, I'm in no specific rush. I've just been mostly trying to get as many conversions done as possible to get them in front of the right people for review. > Sorry for the noise, What I did in many other non-trivial conversions was just add an explicit pointer back, since that's operationally identical to what struct timer_list was storing in its .data field. i.e. add: struct cardstate *cs; to struct bas_cardstate, and then use this on timer entry: struct bas_cardstate *ucs = from_timer(ucs, t, $timer...); struct cardstate *cs = ucs->cs; and this at init: spin_lock_init(&ucs->lock); + ucs->cs = cs; - setup_timer(&ucs->timer_ctrl, req_timeout, (unsigned long) cs); - setup_timer(&ucs->timer_atrdy, atrdy_timeout, (unsigned long) cs); - setup_timer(&ucs->timer_cmd_in, cmd_in_timeout, (unsigned long) cs); - setup_timer(&ucs->timer_int_in, int_in_resubmit, (unsigned long) cs); + timer_setup(&ucs->timer_ctrl, req_timeout, 0); + timer_setup(&ucs->timer_atrdy, atrdy_timeout, 0); + timer_setup(&ucs->timer_cmd_in, cmd_in_timeout, 0); + timer_setup(&ucs->timer_int_in, int_in_resubmit, 0); which will avoid the urb entirely. Do you want me to send an alternative patch? -Kees
On Thu, 2017-10-19 at 23:31 +0200, Thomas Gleixner wrote: > bas_gigaset_exit() > { > for (i = 0; i < driver->minors; i++) { > if (gigaset_shutdown(driver->cs + i) < 0) > > gigaset_shutdown(cs) > { > mutex_lock(&cs->mutex); <-------- Explodes here > > So driver->cs + i is invalid. No idea how that might be related to that > timer conversion patch, but .... Thanks for peeking into this! Please note that driver->minors is one of the more embarrassing warts of the gigaset code. It's basically hardcoded to 1 for all three drivers (including bas_gigaset). So driver->cs itself is invalid here. And since the patch uses struct cardstate *cs = urb->context; in a few places my guess is that it's really the patch that triggers this. Thanks, Paul Bolle
On Thu, 2017-10-19 at 14:31 -0700, Kees Cook wrote: > What I did in many other non-trivial conversions was just add an > explicit pointer back, since that's operationally identical to what > struct timer_list was storing in its .data field. > > i.e. > > add: > > struct cardstate *cs; > > to struct bas_cardstate, and then use this on timer entry: > > struct bas_cardstate *ucs = from_timer(ucs, t, $timer...); > struct cardstate *cs = ucs->cs; That crossed my mind too. (Honestly!) It _feels_ a bit dirty, as I have this idea that structures having references to each other is some sort of an anti- pattern. On the other hand: the various structures used here are, well, not that clean already so I might as well ignore my feelings. > and this at init: > > spin_lock_init(&ucs->lock); > + ucs->cs = cs; > - setup_timer(&ucs->timer_ctrl, req_timeout, (unsigned long) cs); > - setup_timer(&ucs->timer_atrdy, atrdy_timeout, (unsigned long) cs); > - setup_timer(&ucs->timer_cmd_in, cmd_in_timeout, (unsigned long) cs); > - setup_timer(&ucs->timer_int_in, int_in_resubmit, (unsigned long) cs); > + timer_setup(&ucs->timer_ctrl, req_timeout, 0); > + timer_setup(&ucs->timer_atrdy, atrdy_timeout, 0); > + timer_setup(&ucs->timer_cmd_in, cmd_in_timeout, 0); > + timer_setup(&ucs->timer_int_in, int_in_resubmit, 0); > > which will avoid the urb entirely. > > Do you want me to send an alternative patch? That would be nice! Please allow a few days for testing. That testing is beyond silly, though. It requires me getting a laptop very close to the awkward place where my ISDN setup lives. I'll spare you the details. But that silliness again shows, I'd say, that the gigaset code mainly exists to see if there's still a pulse in mainline ISDN. Is that enough to bother? Or should mainline ISDN go the way of, say, IRDA? But I digress. An alternative patch would be much appreciated. Thanks, Paul Bolle
On Thu, 19 Oct 2017, Paul Bolle wrote: > On Thu, 2017-10-19 at 23:31 +0200, Thomas Gleixner wrote: > > bas_gigaset_exit() > > { > > for (i = 0; i < driver->minors; i++) { > > if (gigaset_shutdown(driver->cs + i) < 0) > > > > gigaset_shutdown(cs) > > { > > mutex_lock(&cs->mutex); <-------- Explodes here > > > > So driver->cs + i is invalid. No idea how that might be related to that > > timer conversion patch, but .... > > Thanks for peeking into this! > > Please note that driver->minors is one of the more embarrassing warts of the > gigaset code. It's basically hardcoded to 1 for all three drivers (including > bas_gigaset). So driver->cs itself is invalid here. > > And since the patch uses > struct cardstate *cs = urb->context; > > in a few places my guess is that it's really the patch that triggers this. Well, that does not explain why drivers->cs + i would be corrupted. That would require that this cs -> urb link points at driver magically and then wreckages that driver data structure. Might be the case, but if so then there are dragons burried somehwere Thanks, tglx
[CC-ing Linus because I quote him.] On Fri, 2017-10-20 at 00:28 +0200, Thomas Gleixner wrote: > Well, that does not explain why > > drivers->cs + i > > would be corrupted. That would require that this cs -> urb link points at > driver magically and then wreckages that driver data structure. Might be > the case, but if so then there are dragons burried somehwere Let's assume dragons are buried somewhere. We need users to show us that they met a dragon, right? (I care little about dragons no-one ever stumbles upon.) In the explanation of commit 9f5af546e6ac ("isdn/i4l: fix buffer overflow") Linus added: [ ISDN seems to be effectively unmaintained, and the I4L driver in particular is long deprecated, but in case somebody uses this.. - Linus ] ISDN is pretty niche. So it's no surprise that in mainline it's divided into three parts: I4L, CAPI, and mISDN. Arnd Bergmann has suggested more than once to move I4L to staging. (As far as I know, moving drivers to staging effectively means removing those drivers, but anyhow.) I'd say we'd just should do that. The stuff has been deemed deprecated since basically forever. I never cared about mISDN, but as far as I can see mISDN has quietly left mainline. The only actively maintained CAPI drivers are gigaset's drivers. But I'm afraid maintaining gigaset basically means seeing treewide cleanups fly by and keeping the various fuzzers happy. I don't mind, and I could keep on doing that for years. But still, I'd love to hear someone say: yes, I still care about mainline ISDN. Does that person still exists? Thanks, Paul Bolle
diff --git a/drivers/isdn/gigaset/bas-gigaset.c b/drivers/isdn/gigaset/bas-gigaset.c index 33151f05e744..c990c6bbffc2 100644 --- a/drivers/isdn/gigaset/bas-gigaset.c +++ b/drivers/isdn/gigaset/bas-gigaset.c @@ -433,10 +433,11 @@ static void check_pending(struct bas_cardstate *ucs) * argument: * controller state structure */ -static void cmd_in_timeout(unsigned long data) +static void cmd_in_timeout(struct timer_list *t) { - struct cardstate *cs = (struct cardstate *) data; - struct bas_cardstate *ucs = cs->hw.bas; + struct bas_cardstate *ucs = from_timer(ucs, t, timer_cmd_in); + struct urb *urb = ucs->urb_int_in; + struct cardstate *cs = urb->context; int rc; if (!ucs->rcvbuf_size) { @@ -639,10 +640,11 @@ static void int_in_work(struct work_struct *work) * argument: * controller state structure */ -static void int_in_resubmit(unsigned long data) +static void int_in_resubmit(struct timer_list *t) { - struct cardstate *cs = (struct cardstate *) data; - struct bas_cardstate *ucs = cs->hw.bas; + struct bas_cardstate *ucs = from_timer(ucs, t, timer_int_in); + struct urb *urb = ucs->urb_int_in; + struct cardstate *cs = urb->context; int rc; if (ucs->retry_int_in++ >= BAS_RETRY) { @@ -1441,10 +1443,11 @@ static void read_iso_tasklet(unsigned long data) * argument: * controller state structure */ -static void req_timeout(unsigned long data) +static void req_timeout(struct timer_list *t) { - struct cardstate *cs = (struct cardstate *) data; - struct bas_cardstate *ucs = cs->hw.bas; + struct bas_cardstate *ucs = from_timer(ucs, t, timer_ctrl); + struct urb *urb = ucs->urb_int_in; + struct cardstate *cs = urb->context; int pending; unsigned long flags; @@ -1837,10 +1840,11 @@ static void write_command_callback(struct urb *urb) * argument: * controller state structure */ -static void atrdy_timeout(unsigned long data) +static void atrdy_timeout(struct timer_list *t) { - struct cardstate *cs = (struct cardstate *) data; - struct bas_cardstate *ucs = cs->hw.bas; + struct bas_cardstate *ucs = from_timer(ucs, t, timer_atrdy); + struct urb *urb = ucs->urb_int_in; + struct cardstate *cs = urb->context; dev_warn(cs->dev, "timeout waiting for HD_READY_SEND_ATDATA\n"); @@ -2213,10 +2217,10 @@ static int gigaset_initcshw(struct cardstate *cs) } spin_lock_init(&ucs->lock); - setup_timer(&ucs->timer_ctrl, req_timeout, (unsigned long) cs); - setup_timer(&ucs->timer_atrdy, atrdy_timeout, (unsigned long) cs); - setup_timer(&ucs->timer_cmd_in, cmd_in_timeout, (unsigned long) cs); - setup_timer(&ucs->timer_int_in, int_in_resubmit, (unsigned long) cs); + timer_setup(&ucs->timer_ctrl, req_timeout, 0); + timer_setup(&ucs->timer_atrdy, atrdy_timeout, 0); + timer_setup(&ucs->timer_cmd_in, cmd_in_timeout, 0); + timer_setup(&ucs->timer_int_in, int_in_resubmit, 0); init_waitqueue_head(&ucs->waitqueue); INIT_WORK(&ucs->int_in_wq, int_in_work); diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c index 7c7814497e3e..15482c5de33c 100644 --- a/drivers/isdn/gigaset/common.c +++ b/drivers/isdn/gigaset/common.c @@ -153,9 +153,9 @@ static int test_timeout(struct at_state_t *at_state) return 1; } -static void timer_tick(unsigned long data) +static void timer_tick(struct timer_list *t) { - struct cardstate *cs = (struct cardstate *) data; + struct cardstate *cs = from_timer(cs, t, timer); unsigned long flags; unsigned channel; struct at_state_t *at_state; @@ -687,7 +687,7 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels, cs->ignoreframes = ignoreframes; INIT_LIST_HEAD(&cs->temp_at_states); cs->running = 0; - init_timer(&cs->timer); /* clear next & prev */ + timer_setup(&cs->timer, timer_tick, 0); spin_lock_init(&cs->ev_lock); cs->ev_tail = 0; cs->ev_head = 0; @@ -768,7 +768,6 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels, spin_lock_irqsave(&cs->lock, flags); cs->running = 1; spin_unlock_irqrestore(&cs->lock, flags); - setup_timer(&cs->timer, timer_tick, (unsigned long) cs); cs->timer.expires = jiffies + msecs_to_jiffies(GIG_TICK); add_timer(&cs->timer);
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. Cc: Paul Bolle <pebolle@tiscali.nl> Cc: Karsten Keil <isdn@linux-pingi.de> Cc: "David S. Miller" <davem@davemloft.net> Cc: Johan Hovold <johan@kernel.org> Cc: gigaset307x-common@lists.sourceforge.net Cc: netdev@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/isdn/gigaset/bas-gigaset.c | 36 ++++++++++++++++++++---------------- drivers/isdn/gigaset/common.c | 7 +++---- 2 files changed, 23 insertions(+), 20 deletions(-)