NetBSD Problem Report #51418
From www@NetBSD.org Wed Aug 17 11:36:32 2016
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.netbsd.org", Issuer "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id D3CDA7A266
for <gnats-bugs@gnats.NetBSD.org>; Wed, 17 Aug 2016 11:36:32 +0000 (UTC)
Message-Id: <20160817113631.729737A2D5@mollari.NetBSD.org>
Date: Wed, 17 Aug 2016 11:36:31 +0000 (UTC)
From: joseyluis@gmail.com
Reply-To: joseyluis@gmail.com
To: gnats-bugs@NetBSD.org
Subject: Fix incore src/sbin/fsck_lfs/bufcache.c
X-Send-Pr-Version: www-1.0
>Number: 51418
>Category: bin
>Synopsis: Fix incore src/sbin/fsck_lfs/bufcache.c
>Confidential: no
>Severity: critical
>Priority: medium
>Responsible: dholland
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Wed Aug 17 11:40:00 +0000 2016
>Closed-Date: Sat Jun 09 21:38:56 +0000 2018
>Last-Modified: Thu Jun 14 19:40:01 +0000 2018
>Originator: Jose Luis Rodriguez Garcia
>Release: current and previous releases
>Organization:
>Environment:
>Description:
incore function from buffercache.c uses int instead of daddr_t
it must be: incore(struct uvnode * vp, int lbn)
instead of: incore(struct uvnode * vp, daddr_t lbn)
It produces that daddr_t (that is unint64_t) is casted as int.
In 32 bits platforms (i386 at last), it produces that adress > 1 Tbyte (assuming a 512 DEV_BSIZE), are converted to a negative number. It produces the next problems in 32 bits architectures:
1- Blocks higher than 1 Tbyte aren't cached in the buffer cache. Its buffers aren't found by the incore function, because the real disk adress is compared against a negative number
2- A unique block is cached in several buffers, becuase incore never founds the block. getblock funtion wastes memory adding buffers that won't be found.
3- Potential corruption: A block marked with B_DELWRI (dirty cache) can be contained in different buffers. What buffer will be written first?
I think that the fix must be pulled to previous releases.
>How-To-Repeat:
code review
>Fix:
I have tested the compilation of the fix with a ./build.sh build
Index: bufcache.c
===================================================================
RCS file: /cvsroot/src/sbin/fsck_lfs/bufcache.c,v
retrieving revision 1.16
diff -u -r1.16 bufcache.c
--- bufcache.c 31 Jul 2016 18:27:26 -0000 1.16
+++ bufcache.c 17 Aug 2016 11:26:49 -0000
@@ -194,7 +194,7 @@
/* Return a buffer if it is in the cache, otherwise return NULL. */
struct ubuf *
-incore(struct uvnode * vp, int lbn)
+incore(struct uvnode * vp, daddr_t lbn)
{
struct ubuf *bp;
int hash, depth;
Index: bufcache.h
===================================================================
RCS file: /cvsroot/src/sbin/fsck_lfs/bufcache.h,v
retrieving revision 1.12
diff -u -r1.12 bufcache.h
--- bufcache.h 29 Mar 2015 19:35:58 -0000 1.12
+++ bufcache.h 17 Aug 2016 11:26:49 -0000
@@ -116,7 +116,7 @@
void bufstats(void);
void buf_destroy(struct ubuf *);
void bremfree(struct ubuf *);
-struct ubuf *incore(struct uvnode *, int);
+struct ubuf *incore(struct uvnode *, daddr_t);
struct ubuf *getblk(struct uvnode *, daddr_t, int);
void bwrite(struct ubuf *);
void brelse(struct ubuf *, int);
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: bin-bug-people->dholland
Responsible-Changed-By: maya@NetBSD.org
Responsible-Changed-When: Sat, 03 Dec 2016 11:20:32 +0000
Responsible-Changed-Why:
more lfs
From: Jose Luis Rodriguez Garcia <joseyluis@gmail.com>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/51418: Fix incore src/sbin/fsck_lfs/bufcache.c
Date: Mon, 5 Dec 2016 15:05:27 +0100
Looking in other pieces of code, it seems that sometimes negative
numbers are used. I did some tests time ago, and this patch broke
somethings.
The treatment of disk address seems inconsistent. In buffcache.c uses
mainly daddr_t (unsigned) and in other parts of the code uses signed
numbers. It seems confussing and prone to errors.
On Wed, Aug 17, 2016 at 1:40 PM, <gnats-admin@netbsd.org> wrote:
> Thank you very much for your problem report.
> It has the internal identification `bin/51418'.
> The individual assigned to look at your
> report is: bin-bug-people.
>
>>Category: bin
>>Responsible: bin-bug-people
>>Synopsis: Fix incore src/sbin/fsck_lfs/bufcache.c
>>Arrival-Date: Wed Aug 17 11:40:00 +0000 2016
>
From: Jose Luis Rodriguez Garcia <joseyluis@gmail.com>
To: gnats-bugs@netbsd.org
Cc:
Subject: Re: bin/51418: Fix incore src/sbin/fsck_lfs/bufcache.c
Date: Wed, 22 Nov 2017 12:39:29 +0100
I have tested this patch one more time and it seems fine. Please commit it.
I couldn't reproduce the previous failures, they must have been problems
with my old QEMU/FreeBSD setup that it isn't enough stable.
I have passed the fs tests, newfs, uncompress several tars and all has
been ok. (doing more operations with the filesystem fails after/before
the patch but these are other bugs of lfs).
This patch is needed because on 32 bits systems (daddr_t is a int64_t)
there will be corruption/slow performance for LFS fileystems > 1 Tbyte
(LFS2 allows up to 2 Tbytes according to the roadmap) and LFS64.
On Mon, Dec 5, 2016 at 3:10 PM, Jose Luis Rodriguez Garcia
<joseyluis@gmail.com> wrote:
> The following reply was made to PR bin/51418; it has been noted by GNATS.
>
> From: Jose Luis Rodriguez Garcia <joseyluis@gmail.com>
> To: gnats-bugs@netbsd.org
> Cc:
> Subject: Re: bin/51418: Fix incore src/sbin/fsck_lfs/bufcache.c
> Date: Mon, 5 Dec 2016 15:05:27 +0100
>
> Looking in other pieces of code, it seems that sometimes negative
> numbers are used. I did some tests time ago, and this patch broke
> somethings.
>
> The treatment of disk address seems inconsistent. In buffcache.c uses
> mainly daddr_t (unsigned) and in other parts of the code uses signed
> numbers. It seems confussing and prone to errors.
>
> On Wed, Aug 17, 2016 at 1:40 PM, <gnats-admin@netbsd.org> wrote:
> > Thank you very much for your problem report.
> > It has the internal identification `bin/51418'.
> > The individual assigned to look at your
> > report is: bin-bug-people.
> >
> >>Category: bin
> >>Responsible: bin-bug-people
> >>Synopsis: Fix incore src/sbin/fsck_lfs/bufcache.c
> >>Arrival-Date: Wed Aug 17 11:40:00 +0000 2016
> >
>
From: Jose Luis Rodriguez Garcia <joseyluis@gmail.com>
To: gnats-bugs@netbsd.org
Cc: David Holland <dholland@netbsd.org>, gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Subject: Re: bin/51418: Fix incore src/sbin/fsck_lfs/bufcache.c
Date: Fri, 30 Mar 2018 04:14:37 +0200
Could the proposed patch in the original bug report patch be?
It is a simple fix that avoids some corruptions in big LFS volumes.
Best Regards
From: David Holland <dholland@netbsd.org>
To: Jose Luis Rodriguez Garcia <joseyluis@gmail.com>
Cc: gnats-bugs@netbsd.org
Subject: Re: bin/51418: Fix incore src/sbin/fsck_lfs/bufcache.c
Date: Fri, 30 Mar 2018 06:50:12 +0000
On Fri, Mar 30, 2018 at 04:14:37AM +0200, Jose Luis Rodriguez Garcia wrote:
> Could the proposed patch in the original bug report patch be?
>
> It is a simple fix that avoids some corruptions in big LFS volumes.
sorry, haven't had time to think about it or much of anything else in
the past 2-3 months :(
--
David A. Holland
dholland@netbsd.org
From: "Christos Zoulas" <christos@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51418 CVS commit: src/sbin/fsck_lfs
Date: Fri, 30 Mar 2018 08:56:46 -0400
Module Name: src
Committed By: christos
Date: Fri Mar 30 12:56:46 UTC 2018
Modified Files:
src/sbin/fsck_lfs: bufcache.c bufcache.h
Log Message:
PR/51418: Jose Luis Rodriguez Garcia: Fix incore src/sbin/fsck_lfs/bufcache.c
XXX: pullup-8, pullup-7
To generate a diff of this commit:
cvs rdiff -u -r1.19 -r1.20 src/sbin/fsck_lfs/bufcache.c
cvs rdiff -u -r1.13 -r1.14 src/sbin/fsck_lfs/bufcache.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: christos@zoulas.com (Christos Zoulas)
To: gnats-bugs@NetBSD.org, dholland@NetBSD.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org, joseyluis@gmail.com
Cc:
Subject: Re: bin/51418: Fix incore src/sbin/fsck_lfs/bufcache.c
Date: Fri, 30 Mar 2018 08:57:06 -0400
On Mar 30, 2:15am, joseyluis@gmail.com (Jose Luis Rodriguez Garcia) wrote:
-- Subject: Re: bin/51418: Fix incore src/sbin/fsck_lfs/bufcache.c
| It is a simple fix that avoids some corruptions in big LFS volumes.
Indeed, thanks for your patience!
christos
State-Changed-From-To: open->pending-pullups
State-Changed-By: maya@NetBSD.org
State-Changed-When: Mon, 04 Jun 2018 08:58:36 +0000
State-Changed-Why:
pullup-8 #855, pullup-7 #1612
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51418 CVS commit: [netbsd-7] src/sbin/fsck_lfs
Date: Wed, 6 Jun 2018 15:38:05 +0000
Module Name: src
Committed By: martin
Date: Wed Jun 6 15:38:05 UTC 2018
Modified Files:
src/sbin/fsck_lfs [netbsd-7]: bufcache.c bufcache.h
Log Message:
Pull up following revision(s) (requested by maya in ticket #1612):
sbin/fsck_lfs/bufcache.h: revision 1.14
sbin/fsck_lfs/bufcache.c: revision 1.20
PR/51418: Jose Luis Rodriguez Garcia: Fix incore src/sbin/fsck_lfs/bufcache.c
XXX: pullup-8, pullup-7
To generate a diff of this commit:
cvs rdiff -u -r1.14 -r1.14.4.1 src/sbin/fsck_lfs/bufcache.c
cvs rdiff -u -r1.11 -r1.11.38.1 src/sbin/fsck_lfs/bufcache.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51418 CVS commit: [netbsd-8] src/sbin/fsck_lfs
Date: Fri, 8 Jun 2018 10:21:12 +0000
Module Name: src
Committed By: martin
Date: Fri Jun 8 10:21:12 UTC 2018
Modified Files:
src/sbin/fsck_lfs [netbsd-8]: bufcache.c bufcache.h
Log Message:
Pull up following revision(s) (requested by maya in ticket #855):
sbin/fsck_lfs/bufcache.h: revision 1.14
sbin/fsck_lfs/bufcache.c: revision 1.20
PR/51418: Jose Luis Rodriguez Garcia: Fix incore src/sbin/fsck_lfs/bufcache.c
XXX: pullup-8, pullup-7
To generate a diff of this commit:
cvs rdiff -u -r1.19 -r1.19.6.1 src/sbin/fsck_lfs/bufcache.c
cvs rdiff -u -r1.13 -r1.13.6.1 src/sbin/fsck_lfs/bufcache.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: pending-pullups->closed
State-Changed-By: maya@NetBSD.org
State-Changed-When: Sat, 09 Jun 2018 21:38:56 +0000
State-Changed-Why:
Pullups completed, thanks for the patch!
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51418 CVS commit: [netbsd-7-1] src/sbin/fsck_lfs
Date: Thu, 14 Jun 2018 19:36:53 +0000
Module Name: src
Committed By: martin
Date: Thu Jun 14 19:36:53 UTC 2018
Modified Files:
src/sbin/fsck_lfs [netbsd-7-1]: bufcache.c bufcache.h
Log Message:
Pull up following revision(s) (requested by maya in ticket #1612):
sbin/fsck_lfs/bufcache.h: revision 1.14
sbin/fsck_lfs/bufcache.c: revision 1.20
PR/51418: Jose Luis Rodriguez Garcia: Fix incore src/sbin/fsck_lfs/bufcache.c
XXX: pullup-8, pullup-7
To generate a diff of this commit:
cvs rdiff -u -r1.14 -r1.14.10.1 src/sbin/fsck_lfs/bufcache.c
cvs rdiff -u -r1.11 -r1.11.44.1 src/sbin/fsck_lfs/bufcache.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51418 CVS commit: [netbsd-7-0] src/sbin/fsck_lfs
Date: Thu, 14 Jun 2018 19:37:56 +0000
Module Name: src
Committed By: martin
Date: Thu Jun 14 19:37:56 UTC 2018
Modified Files:
src/sbin/fsck_lfs [netbsd-7-0]: bufcache.c bufcache.h
Log Message:
Pull up following revision(s) (requested by maya in ticket #1612):
sbin/fsck_lfs/bufcache.h: revision 1.14
sbin/fsck_lfs/bufcache.c: revision 1.20
PR/51418: Jose Luis Rodriguez Garcia: Fix incore src/sbin/fsck_lfs/bufcache.c
XXX: pullup-8, pullup-7
To generate a diff of this commit:
cvs rdiff -u -r1.14 -r1.14.6.1 src/sbin/fsck_lfs/bufcache.c
cvs rdiff -u -r1.11 -r1.11.40.1 src/sbin/fsck_lfs/bufcache.h
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.