Date: Mon, 26 May 2008 20:49:03 +0000 (UTC) From: "Bjoern A. Zeeb" <bzeeb-lists@lists.zabbadoz.net> To: Michael Reifenberger <mr@FreeBSD.org> Cc: cvs-src@FreeBSD.org, src-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/usr.sbin/jexec jexec.8 jexec.c Message-ID: <20080526202423.N65662@maildrop.int.zabbadoz.net> In-Reply-To: <200805261157.m4QBvnpF025029@repoman.freebsd.org> References: <200805261157.m4QBvnpF025029@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 26 May 2008, Michael Reifenberger wrote:
Hi,
> mr 2008-05-26 11:57:49 UTC
>
> FreeBSD src repository
>
> Modified files:
> usr.sbin/jexec jexec.8 jexec.c
> Log:
> Extend jexec to accept hostname or ip-number besides jail-id.
>
> MFC after: 2 weeks
>
> Revision Changes Path
> 1.5 +7 -2 src/usr.sbin/jexec/jexec.8
> 1.5 +53 -2 src/usr.sbin/jexec/jexec.c
It seems you decided to leave this in (for now).
So here are my problems with the code - could you please fix them?
1. you are accepting an IP address or a hostname instead of a jail ID
without an option letter like -i or -h. Why is this a problem?
I have hostnames like 127.
127 is a valid IP Address.
127 could be a Jail-ID I am not refereing to.
2. When going through the list of IPs you got from the kernel:
+ for (i = 0; i < len / sizeof(*xp); i++) {
+ in.s_addr = ntohl(xp->pr_ip);
it should be htonl. (at least for correct documentation purposes)
3. You are comparing strings of IP addresses:
+ if ((strncmp(inet_ntoa(in), addr, slen) == 0) ||
Now, would that match 127 or 0x7f000000? No. Convert the string
to an address and compare the numbers.
4. You are taking the length og the input address:
+ slen = strlen(addr);
Assume I gave 192.0.2.1 as input, the length would be 9.
Assume the kernel gives you back 192.0.2.111.
strncmp on those two would give you a match, which is wrong.
Same is true for hostnames, like input "foo", kernel "foobar".
5. addr2jid should be static
6. On match you are doing:
+ free(sxp);
+ return (xp->pr_id);
freeing sxp but returning xp->pr_id but
wherever xp points to is within the malloced area of sxp.
Use after free.
Greetings
Bjoern
--
Bjoern A. Zeeb Stop bit received. Insert coin for new game.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20080526202423.N65662>