Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754733AbYLISFU (ORCPT ); Tue, 9 Dec 2008 13:05:20 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753699AbYLISFE (ORCPT ); Tue, 9 Dec 2008 13:05:04 -0500 Received: from ug-out-1314.google.com ([66.249.92.172]:55092 "EHLO ug-out-1314.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753547AbYLISFA (ORCPT ); Tue, 9 Dec 2008 13:05:00 -0500 DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=message-id:date:from:sender:to:subject:cc:in-reply-to:mime-version :content-type:content-transfer-encoding:content-disposition :references:x-google-sender-auth; b=HZuXFzvnF7eySpq79DHEwHs4khIsx4pJ+kH2zxjCzSI1bzvlMI0EuAcNfvLWwoalxw jb+c2bO5Vj8m5+uoAtpUhDGGZj8FrieLi1O09n+cvHoAolJnmMSM4USjIoLvoZGjZO4y 0sZVONM5jZvMkSt3pyqRbKC/zONyYK5K6DKQg= Message-ID: Date: Tue, 9 Dec 2008 18:04:58 +0000 From: "Duane Griffin" To: "Boaz Harrosh" Subject: Re: Checking link targets are NULL-terminated Cc: linux-kernel , linux-fsdevel@vger.kernel.org, linux-ext4@vger.kernel.org, "Andrew Morton" , "Christoph Hellwig" In-Reply-To: <493EA0E4.10604@panasas.com> MIME-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Content-Disposition: inline References: <20081205144810.GA25585@dastardly.home.dghda.com> <493E8F31.3040806@panasas.com> <493EA0E4.10604@panasas.com> X-Google-Sender-Auth: 779d43332200c65f Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2406 Lines: 51 2008/12/9 Boaz Harrosh : > I just want to make sure that you understand the code above and convince > you that this can/should be done and will damage nothing. No problem, it never hurts to spell things out in detail :) > The code you see above is only for links that are shorter then some constant. > The ext2 (and other fs's) will cache this case and write the symlink directly > into the inode that will then have 0 number of data blocks. The space allocated > at inode is constant and is chosen for good inode packing on disk. The inode > starts empty then if a symlink is short the string is strcpy to above buffer. Sure, I understand this. > So even if intentional damage was done to on-disk data, putting another null > at the end will never hurt. At most it is redundant since there is another > one preceding. But in the case of damage the damage is fixed. There can never > be an information lost. If the link name on disk is corrupted and the last character is not a zero then it may be changed. That is information lost, albeit probably not *useful* information. I wouldn't like to make such a change without an OK from the FS maintainers, but I'll happily do it with one. So far we seem to have one vote in favour and none against. :) Note that the corruption doesn't have to be intentional, of course, although it was in the particular bug report I was looking at originally. > For symlinks that are longer then above constant 1 data block is allocated > and the symlink is written, padded by zeros. This is taken care of by the > generic layer in the code you patched at fs/namei.c. Terminating at > i_size + 1 will never reach the disk since only i_size bytes are ever written. If the data on disk is corrupted such that i_size == PAGE_SIZE then again we would have different data in-memory and on-disk. However in this case the page would only contain the link name and so wouldn't be dirtied and written out unless the name was changed anyway. So I agree, it should be safe in this case, regardless of my concerns above. Thanks, Duane. -- "I never could learn to drink that blood and call it wine" - Bob Dylan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/