--- El vie, 11/9/09, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> escribió:
> > Some manufacturers provide vendor information in
> non-vendor specific CIS
> > tuples. For example, Broadcom uses an Extended
> Function tuple to provide
> > the MAC address on some of their network cards, as in
> the case of the
> > Nintendo Wii WLAN daughter card.
> >
> > This patch allows passing correct tuples unknown to
> the SDIO core to
> > a matching SDIO driver instead of rejecting them and
> failing.
> >
>
> This looks leaky to me.
>
Hi Andrew, thanks for the review.
I hope I can clarify a bit what the patch does in the next comments.
>
> : if (i < ARRAY_SIZE(cis_tpl_list)) {
> : const struct cis_tpl *tpl = cis_tpl_list + i;
> : if (tpl_link < tpl->min_size) {
> : printk(KERN_ERR
> : "%s: bad CIS tuple 0x%02x"
> : " (length = %u, expected >= %u)\n",
> : mmc_hostname(card->host),
> : tpl_code, tpl_link, tpl->min_size);
> : ret = -EINVAL;
>
> ret == -EINVAL
>
At this point ret is not -EINVAL. If it was -EINVAL the code would have had exit at this snipped before:
if (ret) {
kfree(this);
break;
}
> : } else if (tpl->parse) {
> : ret = tpl->parse(card, func,
> : this->data, tpl_link);
> : }
> : /* already successfully parsed, not needed anymore */
> : if (!ret)
> : kfree(this);
>
> `this' doesn't get freed
>
Yes, that's the whole point of the patch. It must be freed _only_ if the SDIO core parsed it. If the SDIO core cannot parse it then it gets passed to the SDIO driver via the SDIO func struct tuple list (later).
> : } else {
> : /* unknown tuple */
> : ret = -EILSEQ;
> : }
> :
> : if (ret == -EILSEQ) {
>
> `this' doesn't get remembered.
>
When ret is -EILSEQ `this' is linked to the SDIO func tuple list (later).
> : /* this tuple is unknown to the core */
> : this->next = NULL;
> : this->code = tpl_code;
> : this->size = tpl_link;
> : *prev = this;
> : prev = &this->next;
> : pr_debug("%s: queuing CIS tuple 0x%02x length %u\n",
> : mmc_hostname(card->host), tpl_code, tpl_link);
> : /* keep on analyzing tuples */
> : ret = 0;
> : }
> :
> : ptr += tpl_link;
>
> `this' leaks.
>
`this' doesn't leak. `this' has been linked to the SDIO func tuple list in:
*prev = this;
> : } while (!ret);
>
> Please check?
>
Thanks a lot for you comments,
Albert
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
