[PATCH] btrfs compression: check string length
AFAICT, with current code user could pass something like "lzox" and
still get "lzo" compression. Check string lengths to prevent that.
Signed-off-by: Pavel Machek
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index b05b361..1083ab4 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -51,7 +51,7 @@ bool btrfs_compress_is_valid_type(const char *str, size_t len)
for (i = 1; i < ARRAY_SIZE(btrfs_compress_types); i++) {
size_t comp_len = strlen(btrfs_compress_types[i]);
- if (len < comp_len)
+ if (len != comp_len)
continue;
if (!strncmp(btrfs_compress_types[i], str, comp_len))
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
signature.asc
Description: Digital signature
Re: [PATCH][RFC]: mutex: adaptive spin
> Linus, what do you think about this particular approach of spin-mutexes? > It's not the typical spin-mutex i think. > > The thing i like most about Peter's patch (compared to most other adaptive > spinning approaches i've seen, which all sucked as they included various > ugly heuristics complicating the whole thing) is that it solves the "how > long should we spin" question elegantly: we spin until the owner runs on a > CPU. Well; if there's a timeout, that's obviously safe. But this has no timeout, and Linus wants to play games with accessing 'does owner run on cpu?' lockless. Now, can it mistakenly spin when the owner is scheduled away? That would deadlock, and without locking, I'm not sure if we prevent that -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning and BUG with btrfs and corrupted image
On Tue 2009-01-13 15:43:07, Eric Sesterhenn wrote: > * Chris Mason (chris.ma...@oracle.com) wrote: > > On Tue, 2009-01-13 at 15:21 +0100, Eric Sesterhenn wrote: > > > Hi, > > > > > > when mounting an intentionally corrupted btrfs filesystem i get the > > > following warning and bug message. The image can be found here > > > www.cccmz.de/~snakebyte/btrfs.2.img.bck.bz2 > > > > Thanks for looking at things > > > > Aside from catching checksumming errors, we're not quite ready for > > fuzzer style attacks. The code will be hardened for this but it isn't > > yet. > > Does this mean i should stop trying to break it for now or are you interested > in further reports? Does ext2/3 and vfat survive that kind of attacks? Those are 'in production' and should survive it... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning and BUG with btrfs and corrupted image
Hi! > > > > Thanks for looking at things > > > > > > > > Aside from catching checksumming errors, we're not quite ready for > > > > fuzzer style attacks. The code will be hardened for this but it isn't > > > > yet. > > > > > > Does this mean i should stop trying to break it for now or are you > > > interested > > > in further reports? > > > > Does ext2/3 and vfat survive that kind of attacks? Those are 'in > > production' and should survive it... > > I regularly (once or twice a week) test 100 corrupted images of > vfat, udf, msdos, swap, iso9660, ext2, ext3, ext4, minix, bfs, befs, > hfs, hfs+, qnx4, affs and cramfs on each of my two test machines. > > They are all pretty stable, one remaining thing on my list i didnt have > time to look into was an issue with fat (msdos) triggering a bug in > buffer.c the other is a warning with ext4 in jbd2/checkpoint.c:166 Good, I did not expect filesystems to be in so good state. Thanks! Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning and BUG with btrfs and corrupted image
On Tue 2009-01-20 18:34:55, Eric Sesterhenn wrote: > * Dave Chinner (da...@fromorbit.com) wrote: > > On Tue, Jan 20, 2009 at 11:15:03AM +0100, Eric Sesterhenn wrote: > > > * Dave Chinner (da...@fromorbit.com) wrote: > > > > On Tue, Jan 20, 2009 at 07:31:50AM +0100, Eric Sesterhenn wrote: > > > > > * Pavel Machek (pa...@suse.cz) wrote: > > > > > > Does ext2/3 and vfat survive that kind of attacks? Those are 'in > > > > > > production' and should survive it... > > > > > > > > > > I regularly (once or twice a week) test 100 corrupted images of > > > > > vfat, udf, msdos, swap, iso9660, ext2, ext3, ext4, minix, bfs, befs, > > > > > hfs, hfs+, qnx4, affs and cramfs on each of my two test machines. > > > > > > > > Any reason you are not testing XFS in that set? > > > > > > So far the responses from xfs folks have been disappointing, if you are > > > interested in bugreports i can send you some. > > > > Sure I am. It would be good if you could start testing XFS along > > with all the other filesystems and report anything you find. > > Ok, i wont report stuff with only xfs-internal backtraces from > xfs_error_report() or are they interesting to you? > > This occurs during mount, box is dead afterwards > Image can be found here : > http://www.cccmz.de/~snakebyte/xfs.11.img.bz2 > I see this every ~10 images, which makes further testing hard :) BTW have you considered trying to run fsck.* on such images? I had lot of fun with e2fsck/e3fsck/fsck.vfat. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning and BUG with btrfs and corrupted image
On Tue 2009-01-20 08:28:29, Christoph Hellwig wrote: > On Tue, Jan 20, 2009 at 11:59:44PM +1100, Dave Chinner wrote: > > > So far the responses from xfs folks have been disappointing, if you are > > > interested in bugreports i can send you some. > > > > Sure I am. It would be good if you could start testing XFS along > > with all the other filesystems and report anything you find. > > I think that was the issue with the debug builds. If you do this > testing always do it without CONFIG_XFS_DEBUG set as with that option > we intentionally panic on detected disk corruptions. Uhuh, *_DEBUG options are not supposed to make kernel less stable/robust. Should that crashing functionality be guarded with command line option or something? ext2 has errors=panic mount option... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning and BUG with btrfs and corrupted image
On Wed 2009-01-21 15:00:42, Dave Chinner wrote: > On Tue, Jan 20, 2009 at 11:20:19PM +0100, Pavel Machek wrote: > > On Tue 2009-01-20 08:28:29, Christoph Hellwig wrote: > > > On Tue, Jan 20, 2009 at 11:59:44PM +1100, Dave Chinner wrote: > > > > > So far the responses from xfs folks have been disappointing, if you > > > > > are > > > > > interested in bugreports i can send you some. > > > > > > > > Sure I am. It would be good if you could start testing XFS along > > > > with all the other filesystems and report anything you find. > > > > > > I think that was the issue with the debug builds. If you do this > > > testing always do it without CONFIG_XFS_DEBUG set as with that option > > > we intentionally panic on detected disk corruptions. > > > > Uhuh, *_DEBUG options are not supposed to make kernel less > > stable/robust. Should that crashing functionality be guarded with > > command line option or something? ext2 has errors=panic mount > > option... > > No, it's a debugging option that is described as: > > "Say N unless you are an XFS developer, or you play one on TV." > > Seriously, if you aren't trying to develop XFS stuff then *don't turn it > on*. What about this, then? Pavel --- Warn in documentation that XFS_DEBUG panics machines. Signed-off-by: Pavel Machek diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig index 3f53dd1..55c98eb 100644 --- a/fs/xfs/Kconfig +++ b/fs/xfs/Kconfig @@ -76,4 +76,7 @@ config XFS_DEBUG Note that the resulting code will be HUGE and SLOW, and probably not useful unless you are debugging a particular problem. + Turning this option on will result in kernel panicking any time + it detects on-disk corruption. + Say N unless you are an XFS developer, or you play one on TV. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning and BUG with btrfs and corrupted image
On Sun 2009-02-01 12:40:50, Dave Chinner wrote: > On Mon, Jan 26, 2009 at 05:27:11PM +0100, Pavel Machek wrote: > > On Wed 2009-01-21 15:00:42, Dave Chinner wrote: > > > On Tue, Jan 20, 2009 at 11:20:19PM +0100, Pavel Machek wrote: > > > > On Tue 2009-01-20 08:28:29, Christoph Hellwig wrote: > > > > > I think that was the issue with the debug builds. If you do this > > > > > testing always do it without CONFIG_XFS_DEBUG set as with that option > > > > > we intentionally panic on detected disk corruptions. > > > > > > > > Uhuh, *_DEBUG options are not supposed to make kernel less > > > > stable/robust. Should that crashing functionality be guarded with > > > > command line option or something? ext2 has errors=panic mount > > > > option... > > > > > > No, it's a debugging option that is described as: > > > > > > "Say N unless you are an XFS developer, or you play one on TV." > > > > > > Seriously, if you aren't trying to develop XFS stuff then *don't turn it > > > on*. > > > > What about this, then? > > > > > + Turning this option on will result in kernel panicking any time > > + it detects on-disk corruption. > > Thin end of a wedge. There's a couple of thousand conditions that > CONFIG_XFS_DEBUG introduces kernel panics on: > > $ grep -r ASSERT fs/xfs |wc -l > 2095 > > > CONFIG_*_DEBUG means include *debug* code there to help developers, > including adding additional failure tests into the kernel. Besides, > which bit of "don't turn it on unless you are an XFS developer" > don't you understand? Yes, but DEBUG code is normally to help debugging, not to crash kernels. IMO xfs should use errors=panic mount option as ext3 does, but... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning and BUG with btrfs and corrupted image
> > > CONFIG_*_DEBUG means include *debug* code there to help developers,
> > > including adding additional failure tests into the kernel. Besides,
> > > which bit of "don't turn it on unless you are an XFS developer"
> > > don't you understand?
> >
> > Yes, but DEBUG code is normally to help debugging, not to crash
> > kernels.
>
> Crashing the kernel at exactly the point a problem is detected
> is often the simplest way of debugging the problem.
>
> e.g. CONFIG_VM_DEBUG=y turns on VM_BUG_ON() which crashes the kernel
> whenever it detects something wrong. Do I turn it on? Yes. Do i
That's different. User is not supposed to be able to trigger
VM_BUG_ON().
> complain about it when I hit a VM_BUG_ON()? No, I report the
> bug and move on. If you turn on a DEBUG option, then you are
> asking the system to behave in a way useful to a developer,
> not an end user. That includes panicing when something wrong
> is detected.
Imagine vm going panic() on mkdir("/lost+found")...
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning and BUG with btrfs and corrupted image
On Thu 2009-02-05 08:02:39, Chris Mason wrote:
> On Thu, 2009-02-05 at 10:02 +0100, Pavel Machek wrote:
> > > > > CONFIG_*_DEBUG means include *debug* code there to help developers,
> > > > > including adding additional failure tests into the kernel. Besides,
> > > > > which bit of "don't turn it on unless you are an XFS developer"
> > > > > don't you understand?
> > > >
> > > > Yes, but DEBUG code is normally to help debugging, not to crash
> > > > kernels.
> > >
> > > Crashing the kernel at exactly the point a problem is detected
> > > is often the simplest way of debugging the problem.
> > >
> > > e.g. CONFIG_VM_DEBUG=y turns on VM_BUG_ON() which crashes the kernel
> > > whenever it detects something wrong. Do I turn it on? Yes. Do i
> >
> > That's different. User is not supposed to be able to trigger
> > VM_BUG_ON().
> >
> > > complain about it when I hit a VM_BUG_ON()? No, I report the
> > > bug and move on. If you turn on a DEBUG option, then you are
> > > asking the system to behave in a way useful to a developer,
> > > not an end user. That includes panicing when something wrong
> > > is detected.
> >
> > Imagine vm going panic() on mkdir("/lost+found")...
>
> It is up to the XFS developers to decide what their debugging options
> do.
Eh?
> The whole point of panicing is so that you can collect important
> information about the system at the time of the error condition. When
> this option is compiled on, panic on mkdir is exactly what they are
> asking for.
>
> If you don't want it, don't compile it in. The Kconfig text is very
> clear.
No, I'd not expect that option to panic systems. That's why I
suggested:
diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig
index 29228f5..b7ac847 100644
--- a/fs/xfs/Kconfig
+++ b/fs/xfs/Kconfig
@@ -77,4 +77,7 @@ config XFS_DEBUG
Note that the resulting code will be HUGE and SLOW, and probably
not useful unless you are debugging a particular problem.
+ Turning this option on will result in kernel panicking any time
+ it detects on-disk corruption.
+
Say N unless you are an XFS developer, or you play one on TV.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Warning and BUG with btrfs and corrupted image
On Thu 2009-02-05 09:19:28, jim owens wrote: > Pavel Machek wrote: >>> If you don't want it, don't compile it in. The Kconfig text is very >>> clear. >> >> No, I'd not expect that option to panic systems. That's why I >> suggested: >> >> diff --git a/fs/xfs/Kconfig b/fs/xfs/Kconfig >> index 29228f5..b7ac847 100644 >> --- a/fs/xfs/Kconfig >> +++ b/fs/xfs/Kconfig >> @@ -77,4 +77,7 @@ config XFS_DEBUG >>Note that the resulting code will be HUGE and SLOW, and probably >>not useful unless you are debugging a particular problem. >> + Turning this option on will result in kernel panicking any time >> + it detects on-disk corruption. >> + >>Say N unless you are an XFS developer, or you play one on TV. > > If you really want a better warning it should simply be: > > Choosing Y will make XFS panic on survivable events. Can we get that line there? > I understand you may have a concern that "normal users" will select > the debug option by mistake, but I don't think that is realistic. > My experience is they will not build custom debug kernels even if > you beg them to. They will only use the distro build and a > distro should never turn this option on outside their own labs. > > Any non-xfs kernel developer who turns this on and gets > snake bit will only do it once. One bite per developer is one bite too many.. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
