From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [PATCHv2 1/2] cld: fix CLD_INODE_NAME_MAX woes Date: Wed, 03 Feb 2010 17:27:29 -0500 Message-ID: <4B69F851.4010808@garzik.org> References: <1265204756-20122-1-git-send-email-cmccabe@alumni.cmu.edu> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=qrrSIRc/Ek4cJlNe3HRLu8plmYc63zxpmwc++vWQ0vI=; b=wiE1H+VD8QLV/qOPplIWmMwOceAFMf7cPArOvXYoSsFxwzN5COQP67yq8tdlW88y9E 9QV8t4SZhC5wMQPb9F6XtMLIH2OyqcA58x3DT0rkU1bEc5zFGEemRjSUGMuXsBSnP7+p 00fxw10r6Eff7dCHdgiroghZE8qpjlk1myOHk= In-Reply-To: <1265204756-20122-1-git-send-email-cmccabe@alumni.cmu.edu> Sender: hail-devel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii"; format="flowed" To: Colin McCabe Cc: Project Hail List , Pete Zaitcev On 02/03/2010 08:45 AM, Colin McCabe wrote: > When we create a static buffer for an inode name, and treat it like a > null-terminated string, it needs to be of length CLD_INODE_NAME_MAX + 1 so > that it can hold the NULL-terminator. > > In cldc_del and cldc_open, we should check that the user-submitted inode name > is less than or equal to CLD_INODE_NAME_MAX. Formerly we were just checking > that it wasn't too big to fit in the packet. > > When copying the inode name out of struct cld_dirent_cur, use snprintf rather > than strcpy to ensure that we never overflow the buffer. This isn't strictly > necessary if all other checks are working perfectly, but it seems prudent. > > Signed-off-by: Colin McCabe applied, after s/snprintf/strncpy/ In general, too, you should never pass a variable string into snprintf, as that may make a program vulnerable to printf format string attacks (user supplies "%s" as a username, for example). A few other changes made to your XDR work: * "\n" removed from log messages, as that is appended as needed by log implementation * user_key() restored. that is our authentication hook, and it must be called, even though it merely returns the username passed to it at present. * msg type renamed back to msg op Jeff