From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755600AbdJJHdP (ORCPT ); Tue, 10 Oct 2017 03:33:15 -0400 Received: from mx2.suse.de ([195.135.220.15]:44119 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751903AbdJJHdJ (ORCPT ); Tue, 10 Oct 2017 03:33:09 -0400 Date: Tue, 10 Oct 2017 09:33:07 +0200 From: Jan Kara To: Steve Magnani Cc: Jan Kara , linux-kernel@vger.kernel.org, "Steven J . Magnani" Subject: Re: [PATCH] udf: Fix 64-bit sign extension issues affecting blocks > 0x7FFFFFFF Message-ID: <20171010073307.GA775@quack2.suse.cz> References: <1507561492-21504-1-git-send-email-steve@digidescorp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1507561492-21504-1-git-send-email-steve@digidescorp.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon 09-10-17 10:04:52, Steve Magnani wrote: > Large (> 1 TiB) UDF filesystems appear subject to several problems when > mounted on 64-bit systems: > > * readdir() can fail on a directory containing File Identifiers residing > above 0x7FFFFFFF. This manifests as a 'ls' command failing with EIO. > > * FIBMAP on a file block located above 0x7FFFFFFF can return a negative > value. The low 32 bits are correct, but applications that don't mask the > high 32 bits of the result can perform incorrectly. > > * Unsigned values > 0x7FFFFFFF are output as negative numbers in some > driver printks, e.g.: > Partition (0 type 1511) starts at physical 460, block length -1779968542 > > Take care to use "%u" when printing unsigned values and to use unsigned > types to store UDF block addresses. > > Signed-off-by: Steven J. Magnani Thanks for looking into this and for the patch! However the patch seems to be mixing two changes into one which I'd prefer to be separate patches: 1) Changes so that physical block numbers are stored in uint32_t (and accompanying format string changes). Also when doing this, could you please create a dedicated type like typedef uint32_t udf_pblk_t; and use it instead of uint32_t? That way it would be cleaner what's going on. Thanks! 2) Changes fixing signedness in various format strings for various types - put these in a separare patch please. > --- a/fs/udf/balloc.c (revision 26779) > +++ b/fs/udf/balloc.c (working copy) ... > @@ -151,7 +151,7 @@ > bh = bitmap->s_block_bitmap[bitmap_nr]; > for (i = 0; i < count; i++) { > if (udf_set_bit(bit + i, bh->b_data)) { > - udf_debug("bit %ld already set\n", bit + i); > + udf_debug("bit %lu already set\n", bit + i); This change looks wrong - bit and i are signed. However they are ints, not longs, so that should indeed be fixed. Honza -- Jan Kara SUSE Labs, CR