From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932214AbbLNUeb (ORCPT ); Mon, 14 Dec 2015 15:34:31 -0500 Received: from mx2.suse.de ([195.135.220.15]:32973 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753001AbbLNUe3 (ORCPT ); Mon, 14 Dec 2015 15:34:29 -0500 Date: Mon, 14 Dec 2015 21:34:26 +0100 From: Jan Kara To: Vegard Nossum Cc: Jan Kara , linux-kernel@vger.kernel.org, stable@vger.kernel.org, Quentin Casasnovas , Andrew Morton Subject: Re: [PATCH] udf: limit the maximum number of allocation extents Message-ID: <20151214203426.GO8474@quack.suse.cz> References: <1449845656-8974-1-git-send-email-vegard.nossum@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1449845656-8974-1-git-send-email-vegard.nossum@oracle.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri 11-12-15 15:54:16, Vegard Nossum wrote: > Hit this kernel hang too while fuzzing. Please see this as a tentative > patch indicating where the problem is -- I don't really know much about > UDF or what an allocation extent is or whether there are more problems > in the same neighbourhood. It looks like udf_truncate_extents() might > also have a similar problem? > > Signed-off-by: Vegard Nossum > Cc: stable@vger.kernel.org > Cc: Jan Kara > Cc: Quentin Casasnovas > Cc: Andrew Morton > --- > fs/udf/inode.c | 13 +++++++++++++ > 1 file changed, 13 insertions(+) > > diff --git fs/udf/inode.c fs/udf/inode.c > index 8d0b3ad..e1875f5 100644 > --- fs/udf/inode.c > +++ fs/udf/inode.c > @@ -2047,13 +2047,26 @@ void udf_write_aext(struct inode *inode, struct extent_position *epos, > epos->offset += adsize; > } > > +/* > + * Maximum number of allocation extents. The chosen number is > + * arbitrary - just that we hopefully don't limit any real use > + * but avoid looping for too long on corrupted media. > + */ > +#define UDF_MAX_AEXT_NESTING 4096 > + I don't like to limit the number of indirect extents in a file. Although 4096 is quite a bit, there is a real chance it won't be enough for some usecases (although I agree that such usecases would be very slow with the current implementation of UDF anyway). What I'd prefer is to limit the number of indirect extents to maximum possible sane number. Something like: (inode->i_size >> inode->i_blkbits) / (extents_per_block) + 1 That way we are sure we don't limit any real use case and we also avoid infinite loops. Honza > int8_t udf_next_aext(struct inode *inode, struct extent_position *epos, > struct kernel_lb_addr *eloc, uint32_t *elen, int inc) > { > int8_t etype; > + unsigned int indirections = 0; > > while ((etype = udf_current_aext(inode, epos, eloc, elen, inc)) == > (EXT_NEXT_EXTENT_ALLOCDECS >> 30)) { > + if (++indirections > UDF_MAX_AEXT_NESTING) { > + udf_err(inode->i_sb, "too many AEXTs (max %u supported)\n", UDF_MAX_AEXT_NESTING); > + return -1; > + } > + > int block; > epos->block = *eloc; > epos->offset = sizeof(struct allocExtDesc); > -- > 1.9.1 > > -- Jan Kara SUSE Labs, CR