Re: [PATCH 2/2] drivers/mtd/maps/pcmciamtd.c: coding style cleanups
On Thu, Apr 15, 2010 at 07:27:50PM +0200, Dominik Brodowski wrote:
> On Mon, Apr 12, 2010 at 07:51:12PM +0400, Alexander Kurz wrote:
> > @@ -691,7 +701,8 @@ static void pcmciamtd_detach(struct pcmcia_device *link)
> > if(dev->mtd_info) {
> > del_mtd_device(dev->mtd_info);
> > map_destroy(dev->mtd_info);
> > - info("mtd%d: Removed", dev->mtd_info->index);
> > + dev_info(&dev->p_dev->dev, "mtd%d: Removed\n",
> > +dev->mtd_info->index);
> > }
>
> Could you switch the ordering between map_destroy() and dev_info(), please,
> to avoid an use-after-free? See the attached message by Julia Lawall, who
> noted this issue first.
Actually, to avoid any of the patches getting lost, I created a branch
"pcmciamtd" at:
http://git.kernel.org/?p=linux/kernel/git/brodo/pcmcia-2.6.git;a=shortlog;h=refs/heads/pcmciamtd
___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia
Re: [PATCH 2/2] drivers/mtd/maps/pcmciamtd.c: coding style cleanups
Alexander,
On Mon, Apr 12, 2010 at 07:51:12PM +0400, Alexander Kurz wrote:
> @@ -691,7 +701,8 @@ static void pcmciamtd_detach(struct pcmcia_device *link)
> if(dev->mtd_info) {
> del_mtd_device(dev->mtd_info);
> map_destroy(dev->mtd_info);
> - info("mtd%d: Removed", dev->mtd_info->index);
> + dev_info(&dev->p_dev->dev, "mtd%d: Removed\n",
> + dev->mtd_info->index);
> }
Could you switch the ordering between map_destroy() and dev_info(), please,
to avoid an use-after-free? See the attached message by Julia Lawall, who
noted this issue first.
Best,
Dominik
--- Begin Message ---
From: Julia Lawall
Moved the debugging message before the call to map_destroy, which frees its
argument. The message is also slightly changed to reflect its new
position.
A simplified version of the semantic patch that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
//
@@
expression E,E2;
@@
del_mtd_device(E)
...
(
E = E2
|
* E
)
//
Signed-off-by: Julia Lawall
---
drivers/mtd/maps/pcmciamtd.c|2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/mtd/maps/pcmciamtd.c b/drivers/mtd/maps/pcmciamtd.c
index 87b2b8f..3e339de 100644
--- a/drivers/mtd/maps/pcmciamtd.c
+++ b/drivers/mtd/maps/pcmciamtd.c
@@ -689,8 +689,8 @@ static void pcmciamtd_detach(struct pcmcia_device *link)
if(dev->mtd_info) {
del_mtd_device(dev->mtd_info);
+ info("mtd%d: Removing", dev->mtd_info->index);
map_destroy(dev->mtd_info);
- info("mtd%d: Removed", dev->mtd_info->index);
}
pcmciamtd_release(link);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--- End Message ---
___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia
Re: pccard_validate_cis() vs. antiquated SRAM PCMCIA-Cards
Hey, On Wed, Apr 14, 2010 at 11:36:24PM +0400, Alexander Kurz wrote: > for the pcmciamtd driver there exists a Kconfig option > CONFIG_MTD_PCMCIA_ANONYMOUS to support anonymous storage cards. > At present, using this option has no effect at all, > because anonymous PCMCIA-Cards are filtered out > by pccard_validate_cis() in the pcmcia_card_add() stage. > > Access to this card is possible with the fixed version of the > pcmciamtd driver, when pccard_validate_cis() is disabled. e.g.: > # modprobe pcmciamtd mem_type=1 force_size=1 bankwidth=1 > > Here is an example for a quasi-anonymous card, > a Kingmax (maybe clone) 1MB SRAM-Card SJA-001M5C: > > # pccardctl status > Socket 1: > 5.0V 16-bit PC Card > Subdevice 0 (function 0) bound to driver "pcmciamtd" > # pccardctl info > PRODID_1="" > PRODID_2="" > PRODID_3="" > PRODID_4="" > MANFID=, > FUNCID=64 > > Should I build an exception, or should we declare those > antiquated cards as "not supported"? > An other alternative would be to skip the pccard_validate_cis() > when CONFIG_MTD_PCMCIA_ANONYMOUS is set. > What do you think about this? I don't think it's a good idea to skip the test _in all cases_ if CONFIG_MTD_PCMCIA_ANONYMOUS is set. We should at least be certain that a) either statically mapped sockets are used, or b) the resource setup worked fine. Else we might end up considering a network device (where we can't read the CIS due to the socket not being set up properly) an MTD device. Possibly, forcing an usersapce override (e.g. to a sysfs file) might be needed as well. Best, Dominik ___ Linux PCMCIA reimplementation list http://lists.infradead.org/mailman/listinfo/linux-pcmcia
Re: pci_bus_for_each_resource, transparent bridges and rsrc_nonstatic.c
Thanks, Komuro.
From: Dominik Brodowski
Date: Thu, 15 Apr 2010 19:01:53 +0200
Subject: [PATCH] pcmcia: fix ioport size calculation in rsrc_nonstatic
Size needs to be calculated after manipulating with the start value.
Reported-by: Komuro
Signed-off-by: Dominik Brodowski
diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c
index 1178a82..a6eb7b5 100644
--- a/drivers/pcmcia/rsrc_nonstatic.c
+++ b/drivers/pcmcia/rsrc_nonstatic.c
@@ -810,7 +810,7 @@ static int adjust_memory(struct pcmcia_socket *s, unsigned
int action, unsigned
static int adjust_io(struct pcmcia_socket *s, unsigned int action, unsigned
long start, unsigned long end)
{
struct socket_data *data = s->resource_data;
- unsigned long size = end - start + 1;
+ unsigned long size;
int ret = 0;
#if defined(CONFIG_X86)
@@ -820,6 +820,8 @@ static int adjust_io(struct pcmcia_socket *s, unsigned int
action, unsigned long
start = 0x100;
#endif
+ size = end - start + 1;
+
if (end < start)
return -EINVAL;
___
Linux PCMCIA reimplementation list
http://lists.infradead.org/mailman/listinfo/linux-pcmcia
Re: pci_bus_for_each_resource, transparent bridges and rsrc_nonstatic.c
Hi, After setting start = 0x100, size becomes wrong value... unsigned long size = end - start + 1; <=== HERE int ret = 0; +#if defined(CONFIG_X86) + /* on x86, avoid anything < 0x100 for it is often used for +* legacy platform devices */ + if (start < 0x100) + start = 0x100; <=== HERE +#endif + if (end < start) return -EINVAL; ___ Linux PCMCIA reimplementation list http://lists.infradead.org/mailman/listinfo/linux-pcmcia
Re: [PATCH 1/2] drivers/mtd/maps/pcmciamtd.c: fixing obvious errors
Hi Wolfram, thanks for your comment, On Tue, 13 Apr 2010, Wolfram Sang wrote: > > I'd suggest keeping 'failed', removing the 'err'-statement and using 'goto > failed' in error cases. Currently, pcmciamtd_release() is used many times on > errors. right, sorry, I missed this. Hopefully I will get some more Storage-Cards for further tests next week, so I will post the patch again afterwards. Cheers, Alexander ___ Linux PCMCIA reimplementation list http://lists.infradead.org/mailman/listinfo/linux-pcmcia
