From owner-cvs-all@FreeBSD.ORG Mon May 26 20:49:12 2008 Return-Path: Delivered-To: cvs-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 57833106564A; Mon, 26 May 2008 20:49:12 +0000 (UTC) (envelope-from bzeeb-lists@lists.zabbadoz.net) Received: from mail.cksoft.de (mail.cksoft.de [62.111.66.27]) by mx1.freebsd.org (Postfix) with ESMTP id 097488FC1F; Mon, 26 May 2008 20:49:11 +0000 (UTC) (envelope-from bzeeb-lists@lists.zabbadoz.net) Received: from localhost (amavis.str.cksoft.de [192.168.74.71]) by mail.cksoft.de (Postfix) with ESMTP id 4D29041C749; Mon, 26 May 2008 22:49:10 +0200 (CEST) X-Virus-Scanned: amavisd-new at cksoft.de Received: from mail.cksoft.de ([62.111.66.27]) by localhost (amavis.str.cksoft.de [192.168.74.71]) (amavisd-new, port 10024) with ESMTP id 15U4Vdzqy0yF; Mon, 26 May 2008 22:49:09 +0200 (CEST) Received: by mail.cksoft.de (Postfix, from userid 66) id E28F141C732; Mon, 26 May 2008 22:49:09 +0200 (CEST) Received: from maildrop.int.zabbadoz.net (maildrop.int.zabbadoz.net [10.111.66.10]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mail.int.zabbadoz.net (Postfix) with ESMTP id 74D9D44487F; Mon, 26 May 2008 20:49:03 +0000 (UTC) Date: Mon, 26 May 2008 20:49:03 +0000 (UTC) From: "Bjoern A. Zeeb" X-X-Sender: bz@maildrop.int.zabbadoz.net To: Michael Reifenberger In-Reply-To: <200805261157.m4QBvnpF025029@repoman.freebsd.org> Message-ID: <20080526202423.N65662@maildrop.int.zabbadoz.net> References: <200805261157.m4QBvnpF025029@repoman.freebsd.org> X-OpenPGP-Key: 0x14003F198FEFA3E77207EE8D2B58B8F83CCF1842 MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed 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 X-BeenThere: cvs-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: CVS commit messages for the entire tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 26 May 2008 20:49:12 -0000 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.