linux-kernel - tty, hvc, xen: leak? dead code? everything is fine?

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LNX.2.00.1204122318480.7553@swampdragon.chaosbits.net>
Date:	Thu, 12 Apr 2012 23:33:20 +0200 (CEST)
From:	Jesper Juhl <jj@...osbits.net>
To:	linux-kernel@...r.kernel.org
cc:	Gerd Hoffmann <kraxel@...e.de>, Alan Cox <alan@...ux.intel.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Stefano Stabellini <stefano.stabellini@...citrix.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Subject: tty, hvc, xen: leak? dead code? everything is fine?

Hi

Ok, so I'm staring at drivers/tty/hvc/hvc_xen.c::xen_hvm_console_init() - 
to be specific, this code (and yes, it's late and I'm tired, so I may be 
completely off, I admit that up front):

...
	info = vtermno_to_xencons(HVC_COOKIE);
	if (!info) {
		info = kzalloc(sizeof(struct xencons_info), GFP_KERNEL | 
__GFP_ZERO);
		if (!info)
			return -ENOMEM;
	}

	/* already configured */
	if (info->intfw != NULL)
		return 0;
...

and either I'm completely overlooking something obvious or just being 
stupid (which is certainly possible) or there is a problem here;

If 'info->intfw' is something else than NULL here, then it seems like 
we'll leak the memory we allocated for 'info' when we return (in case 
info started out as NULL and we just allocated it with kzalloc()).

So it seems like we should 'kfree(info)' before the 'return 0' statement, 
but then again, we may not have entered the true branch of 'if (!info)' 
and just allocated 'info' - would it still be right to free it in that 
case?

Or *if* there's actually no way 'info->intfw' can actually be anything but 
NULL in this case, then this would just be dead code (but that doesn't 
seem to be the case).

At the very least, this looks fishy to me... Any feedback is welcome...

-- 
Jesper Juhl <jj@...osbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

--
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/