It is likely that I'm just missing something trivial, but I have
a hard time understanding 63662139e ("params: Fix potential
memory leak in add_sysfs_param()"). [I take it for granted that
krealloc() does not free its first argument when it fails, as
reading mm/slab_common.c seems to confirm.] Some of the relevant
lines from kernel/params.c:
621 /* Enlarge. */
622 new = krealloc(mk->mp,
623 sizeof(*mk->mp) + sizeof(mk->mp->attrs[0]) *
(num+1),
624 GFP_KERNEL);
625 if (!new) {
626 kfree(attrs);
627 err = -ENOMEM;
628 goto fail;
629 }
630 /* Despite looking like the typical realloc() bug, this is safe.
631 * We *want* the old 'attrs' to be freed either way, and we'll
store
632 * the new one in the success case. */
633 attrs = krealloc(attrs, sizeof(new->grp.attrs[0])*(num+2),
GFP_KERNEL);
634 if (!attrs) {
635 err = -ENOMEM;
636 goto fail_free_new;
637 }
[...]
663 fail_free_new:
664 kfree(new);
665 fail:
666 mk->mp = NULL;
667 return err;
First, if the krealloc call on line 622 fails, mk->mp seems to be
leaked (unless the caller happens to have a copy of that pointer
somewhere), since it is NULL'ed on the way out. 63662139e removed
a kfree(mk->mp) call. That doesn't seem right.
Second, the comment seems misleading, again because krealloc()
does not free its argument on failure. So if the krealloc() call
fails, the old attrs does seem to be leaked (attrs may be
mk->mp->grp.attrs, and again mk->mp is NULL'ed on the error
path (not that the caller could use mk->mp for anything - it is
invalidated by the successful first krealloc() call)).
Rasmus
--
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/
