| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <653402b90702122229l525f0d3ak5c5bc0cd7690fdcc@mail.gmail.com>
Date: Tue, 13 Feb 2007 07:29:17 +0100
From: "Miguel Ojeda" <maxextreme@...il.com>
To: "Andrew Morton" <akpm@...ux-foundation.org>
Cc: dwalker@...sta.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH mm] cfag12864b: fix crash when built-in and no parport present
On 2/13/07, Andrew Morton <akpm@...ux-foundation.org> wrote:
> On Sat, 10 Feb 2007 20:51:23 +0100 Miguel Ojeda <maxextreme@...il.com>
> wrote:
>
> > Andrew, Daniel who reported the crash has tested this patch and says that
> it works fine avoiding it, so you can merge.
> >
> >
> drivers-add-lcd-support-fix-crash-when-built-in-and-no-parport-present.patch
> > Signed-off-by: Miguel Ojeda Sandonis <maxextreme@...il.com>
> > ---
> > diff --git a/drivers/auxdisplay/cfag12864b.c
> b/drivers/auxdisplay/cfag12864b.c
> > index 889583d..cb44cb4 100644
> > --- a/drivers/auxdisplay/cfag12864b.c
> > +++ b/drivers/auxdisplay/cfag12864b.c
> > @@ -312,6 +312,17 @@ EXPORT_SYMBOL_GPL(cfag12864b_disable);
> > EXPORT_SYMBOL_GPL(cfag12864b_isenabled);
> >
> > /*
> > + * Is the module inited?
> > + */
> > +
> > +static unsigned char cfag12864b_inited;
> > +unsigned char cfag12864b_isinited(void)
> > +{
> > + return cfag12864b_inited;
> > +}
> > +EXPORT_SYMBOL_GPL(cfag12864b_isinited);
> > +
> > +/*
> > * Module Init & Exit
> > */
> >
> > @@ -319,6 +330,13 @@ static int __init cfag12864b_init(void)
> > {
> > int ret = -EINVAL;
> >
> > + /* ks0108_init() must be called first */
> > + if (!ks0108_isinited()) {
> > + printk(KERN_ERR CFAG12864B_NAME ": ERROR: "
> > + "ks0108 is not initialized\n");
> > + goto none;
> > + }
> > +
> > if (PAGE_SIZE < CFAG12864B_SIZE) {
> > printk(KERN_ERR CFAG12864B_NAME ": ERROR: "
> > "page size (%i) < cfag12864b size (%i)\n",
> > @@ -354,6 +372,7 @@ static int __init cfag12864b_init(void)
> > cfag12864b_clear();
> > cfag12864b_on();
> >
> > + cfag12864b_inited = 1;
> > return 0;
> >
> > cachealloced:
> > diff --git a/drivers/auxdisplay/cfag12864bfb.c
> b/drivers/auxdisplay/cfag12864bfb.c
> > index 94765e7..66fafbb 100644
> > --- a/drivers/auxdisplay/cfag12864bfb.c
> > +++ b/drivers/auxdisplay/cfag12864bfb.c
> > @@ -137,7 +137,14 @@ static struct platform_device *cfag12864
> >
> > static int __init cfag12864bfb_init(void)
> > {
> > - int ret;
> > + int ret = -EINVAL;
> > +
> > + /* cfag12864b_init() must be called first */
> > + if (!cfag12864b_isinited()) {
> > + printk(KERN_ERR CFAG12864BFB_NAME ": ERROR: "
> > + "cfag12864b is not initialized\n");
> > + goto none;
> > + }
> >
> > if (cfag12864b_enable()) {
> > printk(KERN_ERR CFAG12864BFB_NAME ": ERROR: "
> > @@ -162,6 +169,7 @@ static int __init cfag12864bfb_init(void
> > }
> > }
> >
> > +none:
> > return ret;
> > }
> >
> > diff --git a/drivers/auxdisplay/ks0108.c b/drivers/auxdisplay/ks0108.c
> > index a637575..e6c3646 100644
> > --- a/drivers/auxdisplay/ks0108.c
> > +++ b/drivers/auxdisplay/ks0108.c
> > @@ -111,6 +111,17 @@ EXPORT_SYMBOL_GPL(ks0108_address);
> > EXPORT_SYMBOL_GPL(ks0108_page);
> >
> > /*
> > + * Is the module inited?
> > + */
> > +
> > +static unsigned char ks0108_inited;
> > +unsigned char ks0108_isinited(void)
> > +{
> > + return ks0108_inited;
> > +}
> > +EXPORT_SYMBOL_GPL(ks0108_isinited);
> > +
> > +/*
> > * Module Init & Exit
> > */
> >
> > @@ -142,6 +153,7 @@ static int __init ks0108_init(void)
> > goto registered;
> > }
> >
> > + ks0108_inited = 1;
> > return 0;
> >
> > registered:
> > diff --git a/include/linux/cfag12864b.h b/include/linux/cfag12864b.h
> > index 0bc45e6..1605dd8 100644
> > --- a/include/linux/cfag12864b.h
> > +++ b/include/linux/cfag12864b.h
> > @@ -73,5 +73,10 @@ extern void cfag12864b_disable(void);
> > */
> > extern unsigned char cfag12864b_isenabled(void);
> >
> > +/*
> > + * Is the module inited?
> > + */
> > +extern unsigned char cfag12864b_isinited(void);
> > +
> > #endif /* _CFAG12864B_H_ */
> >
> > diff --git a/include/linux/ks0108.h b/include/linux/ks0108.h
> > index 8047d4b..a2c54ac 100644
> > --- a/include/linux/ks0108.h
> > +++ b/include/linux/ks0108.h
> > @@ -43,4 +43,7 @@ extern void ks0108_address(unsigned char
> > /* Set the controller's current page (0..7) */
> > extern void ks0108_page(unsigned char page);
> >
> > +/* Is the module inited? */
> > +extern unsigned char ks0108_isinited(void);
> > +
> > #endif /* _KS0108_H_ */
>
> This looks hacky.
>
> Your patch didn't include a description of the bug (please always include
> such a description) so I'm not sure what's going on here. But I'm
>
Indeed, sorry. The patch is about "crash on CONFIG_CFAG12864B=y in
2.6.20-rc3-mm1" ( http://lkml.org/lkml/2007/1/7/118 ).
> suspecting that module dependencies and appropriate choice of initcall
> levels is the right way to fix whatever bug this is.
>
The problem comes when ks0108/cfag12864b are built-in and any parallel
port is present. ks0108_init() is called first, as it should be, but
fails to load (as there is no parallel port to use).
After that, cfag12864b_init() gets called, without knowing anything
about ks0108 failed, and calls ks0108_writecontrol(), which
dereferences an uninitialized pointer.
Init order is OK, I think. The problem is how to stop
cfag12864b_init() being called if ks0108 failed to load. modprobe does
it for us, but, how when built-in?
As anybody suggested another solution, I send that patch.
Regards,
Miguel
--
Miguel Ojeda
http://maxextreme.googlepages.com/index.htm
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists