lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ac3eb2510810080901g3c3ae2d6l1dd247ed2a1837af@mail.gmail.com>
Date:	Wed, 8 Oct 2008 18:01:19 +0200
From:	"Kay Sievers" <kay.sievers@...y.org>
To:	"Andrew Morton" <akpm@...ux-foundation.org>
Cc:	"Herton Ronaldo Krzesinski" <herton@...driva.com.br>,
	me@...copeland.com, stern@...land.harvard.edu,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	bogdano@...driva.com.br, lcapitulino@...driva.com.br,
	draconux@...il.com, dlallement@...driva.com, pterjan@...driva.com,
	axboe@...nel.dk
Subject: Re: Partition check considered as error is breaking mounting in 2.6.27

On Sat, Sep 13, 2008 at 1:36 AM, Andrew Morton
<akpm@...ux-foundation.org> wrote:
> On Fri, 12 Sep 2008 18:07:27 -0300
> Herton Ronaldo Krzesinski <herton@...driva.com.br> wrote:
>
>> Yes, here goes a new version:
>
> Well gee.  Given a choice, I went and replied to the wrong thread.
> Here's what I think:
>
> From: Andrew Morton <akpm@...ux-foundation.org>
>
> Herton Krzesinski reports that the error-checking changes in
> 04ebd4aee52b06a2c38127d9208546e5b96f3a19 ("block/ioctl.c and
> fs/partition/check.c: check value returned by add_partition") cause his
> buggy USB camera to no longer mount.  "The camera is an Olympus X-840.
> The original issue comes from the camera itself: its format program
> creates a partition with an off by one error".
>
> Buggy devices happen.  It is better for the kernel to warn and to proceed
> with the mount.
>
> Reported-by: Herton Ronaldo Krzesinski <herton@...driva.com.br>
> Cc: Abdel Benamrouche <draconux@...il.com>
> Cc: Jens Axboe <jens.axboe@...cle.com>
> Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> ---
>
>  fs/partitions/check.c |    1 -
>  1 file changed, 1 deletion(-)
>
> diff -puN fs/partitions/check.c~rescan_partitions-make-device-capacity-errors-non-fatal fs/partitions/check.c
> --- a/fs/partitions/check.c~rescan_partitions-make-device-capacity-errors-non-fatal
> +++ a/fs/partitions/check.c
> @@ -540,7 +540,6 @@ int rescan_partitions(struct gendisk *di
>                if (from + size > get_capacity(disk)) {
>                        printk(KERN_ERR " %s: p%d exceeds device capacity\n",
>                                disk->disk_name, p);
> -                       continue;
>                }
>                res = add_partition(disk, p, from, size, state->parts[p].flags);
>                if (res) {

I was happy to see the original fix, as if causes real problems for
userspace, that the kernel creates invalid block devices with a size
that exceeds the physical disk. So, if we can not make that partition
to skip, like original patch did, because of broken hardware we don't
want to break, can we make it at least do the obvious thing, and limit
the partition with the broken entry to the size of the underlying
hardware. So that the kernel does no longer pretend to have devices of
a size which the hardware does not have.

It breaks all sort of userspace tools which read the "size" file in
sysfs, or do BLKGETSIZE and we get, if we are lucky, only:
  attempt to access beyond end of device
  sda: rw=0, want=1953535936, limit=976773168
in other cases it might cause corruption, or lead mkfs to create
devices which will fail when they get used.

Thanks,
Kay
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ