Re: [PATCH 06/10] drivers:ethernet: return -ENOMEM on allocation failure.
From: Allen Pais Date: Wed, 13 Sep 2017 13:02:15 +0530 > Signed-off-by: Allen Pais This is quite pointless as the caller doesn't do anything with the value, it just tests whether a negative value is returned or not. -- 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: crypto: Work around deallocated stack frame reference gcc bug on sparc.
From: David Miller
Date: Fri, 02 Jun 2017 11:28:54 -0400 (EDT)
>
> On sparc, if we have an alloca() like situation, as is the case with
> SHASH_DESC_ON_STACK(), we can end up referencing deallocated stack
> memory. The result can be that the value is clobbered if a trap
> or interrupt arrives at just the right instruction.
>
> It only occurs if the function ends returning a value from that
> alloca() area and that value can be placed into the return value
> register using a single instruction.
>
> For example, in lib/libcrc32c.c:crc32c() we end up with a return
> sequence like:
>
> return %i7+8
> lduw [%o5+16], %o0 ! MEM[(u32 *)__shash_desc.1_10 + 16B],
>
> %o5 holds the base of the on-stack area allocated for the shash
> descriptor. But the return released the stack frame and the
> register window.
>
> So if an intererupt arrives between 'return' and 'lduw', then
> the value read at %o5+16 can be corrupted.
>
> Add a data compiler barrier to work around this problem. This is
> exactly what the gcc fix will end up doing as well, and it absolutely
> should not change the code generated for other cpus (unless gcc
> on them has the same bug :-)
>
> With crucial insight from Eric Sandeen.
>
> Reported-by: Anatoly Pugachev
> Signed-off-by: David S. Miller
Herbert, please take a look at this and push via your crypto tree
and submit to -stable if you don't have any objections.
Thanks!
> ---
>
> See the thread anchored at:
>
> http://marc.info/?l=linux-sparc&m=149623182616944&w=2
>
> for discussion, it has a reproducer module. The problem was
> first noticed as occaisional XFS checksum corruptions.
>
> Herbert, I don't expect you to like this but it is the best we can do
> I think. It should not pessimize code on other architectures at all.
> I will work on fixing the gcc bug but it's been around forever and all
> versions are effected.
>
> I noticed while working on this that at least btrfs duplicates the
> facilities provided by lib/libcrc32c.c and therefore should probably
> be converted over to straight crc32c() calls if possible.
>
> Thanks!
>
> diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
> index ecdba2f..1ac5b85 100644
> --- a/drivers/infiniband/sw/rxe/rxe.h
> +++ b/drivers/infiniband/sw/rxe/rxe.h
> @@ -68,6 +68,7 @@
> static inline u32 rxe_crc32(struct rxe_dev *rxe,
> u32 crc, void *next, size_t len)
> {
> + u32 retval;
> int err;
>
> SHASH_DESC_ON_STACK(shash, rxe->tfm);
> @@ -81,7 +82,9 @@ static inline u32 rxe_crc32(struct rxe_dev *rxe,
> return crc32_le(crc, next, len);
> }
>
> - return *(u32 *)shash_desc_ctx(shash);
> + retval = *(u32 *)shash_desc_ctx(shash);
> + barrier_data(shash_desc_ctx(shash));
> + return retval;
> }
>
> int rxe_set_mtu(struct rxe_dev *rxe, unsigned int dev_mtu);
> diff --git a/fs/btrfs/hash.c b/fs/btrfs/hash.c
> index a97fdc1..baacc18 100644
> --- a/fs/btrfs/hash.c
> +++ b/fs/btrfs/hash.c
> @@ -38,6 +38,7 @@ u32 btrfs_crc32c(u32 crc, const void *address, unsigned int
> length)
> {
> SHASH_DESC_ON_STACK(shash, tfm);
> u32 *ctx = (u32 *)shash_desc_ctx(shash);
> + u32 retval;
> int err;
>
> shash->tfm = tfm;
> @@ -47,5 +48,7 @@ u32 btrfs_crc32c(u32 crc, const void *address, unsigned int
> length)
> err = crypto_shash_update(shash, address, length);
> BUG_ON(err);
>
> - return *ctx;
> + retval = *ctx;
> + barrier_data(ctx);
> + return retval;
> }
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index 2185c7a..fd2e651 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1078,6 +1078,7 @@ static inline u32 f2fs_crc32(struct f2fs_sb_info *sbi,
> const void *address,
> {
> SHASH_DESC_ON_STACK(shash, sbi->s_chksum_driver);
> u32 *ctx = (u32 *)shash_desc_ctx(shash);
> + u32 retval;
> int err;
>
> shash->tfm = sbi->s_chksum_driver;
> @@ -1087,7 +1088,9 @@ static inline u32 f2fs_crc32(struct f2fs_sb_info *sbi,
> const void *address,
> err = crypto_shash_update(shash, address, length);
> BUG_ON(err);
>
> - return *ctx;
> + retval = *ctx;
> + barrier_data(ctx);
> + return retval;
> }
>
> static inline bool f2fs_crc_valid(struct f2fs_sb_info *sbi, __u32 blk_crc,
> diff --git a/lib/libcrc32c.c b/lib/libcrc32c.c
> index 74a54b7..9f79547 100644
> --- a/lib/libcrc32c.c
> +++ b/lib/libcrc32c.c
> @@ -43,7 +43,7 @@ static struct crypto_shash *tfm;
> u
Re: crypto: Work around deallocated stack frame reference gcc bug on sparc.
From: David Miller
Date: Fri, 02 Jun 2017 14:39:06 -0400 (EDT)
> From: "Darrick J. Wong"
> Date: Fri, 2 Jun 2017 11:08:08 -0700
>
>> ext4/jbd2's crc32c implementations will also need a fix like this for
>> {ext4,jbd2}_chksum. Note that both of these modules call the crypto api
>> directly to avoid a static dependence on libcrc32c; this was done to
>> reduce kernel footprint for applications that don't need it. (ext2,
>> ext3, and ext4 before the metadata_csum feature existed).
>
> Good catch. I even looked at that code the other day so should
> have spotted it as well.
>
> I'll integrate fixes for those into my patch when I get a chance,
> thanks!
Actually, ext4 doesn't trigger the problem because the on-stack object
used in ext4 is a fixed size at compile time. Which is technically an
ill-advised assumption to make. Even the generic libcrc32c.c doesn't
assume that the context area is 4 bytes for crc32c.
Anyways, back to the main point, the gcc bug only triggers when
alloca() like constructs are used.
That's why I scanned for SHASH_DESC_ON_STACK() to see exactly where
the workaround is necessary.
--
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: crypto: Work around deallocated stack frame reference gcc bug on sparc.
From: "Darrick J. Wong"
Date: Fri, 2 Jun 2017 11:08:08 -0700
> ext4/jbd2's crc32c implementations will also need a fix like this for
> {ext4,jbd2}_chksum. Note that both of these modules call the crypto api
> directly to avoid a static dependence on libcrc32c; this was done to
> reduce kernel footprint for applications that don't need it. (ext2,
> ext3, and ext4 before the metadata_csum feature existed).
Good catch. I even looked at that code the other day so should
have spotted it as well.
I'll integrate fixes for those into my patch when I get a chance,
thanks!
--
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
crypto: Work around deallocated stack frame reference gcc bug on sparc.
On sparc, if we have an alloca() like situation, as is the case with
SHASH_DESC_ON_STACK(), we can end up referencing deallocated stack
memory. The result can be that the value is clobbered if a trap
or interrupt arrives at just the right instruction.
It only occurs if the function ends returning a value from that
alloca() area and that value can be placed into the return value
register using a single instruction.
For example, in lib/libcrc32c.c:crc32c() we end up with a return
sequence like:
return %i7+8
lduw [%o5+16], %o0 ! MEM[(u32 *)__shash_desc.1_10 + 16B],
%o5 holds the base of the on-stack area allocated for the shash
descriptor. But the return released the stack frame and the
register window.
So if an intererupt arrives between 'return' and 'lduw', then
the value read at %o5+16 can be corrupted.
Add a data compiler barrier to work around this problem. This is
exactly what the gcc fix will end up doing as well, and it absolutely
should not change the code generated for other cpus (unless gcc
on them has the same bug :-)
With crucial insight from Eric Sandeen.
Reported-by: Anatoly Pugachev
Signed-off-by: David S. Miller
---
See the thread anchored at:
http://marc.info/?l=linux-sparc&m=149623182616944&w=2
for discussion, it has a reproducer module. The problem was
first noticed as occaisional XFS checksum corruptions.
Herbert, I don't expect you to like this but it is the best we can do
I think. It should not pessimize code on other architectures at all.
I will work on fixing the gcc bug but it's been around forever and all
versions are effected.
I noticed while working on this that at least btrfs duplicates the
facilities provided by lib/libcrc32c.c and therefore should probably
be converted over to straight crc32c() calls if possible.
Thanks!
diff --git a/drivers/infiniband/sw/rxe/rxe.h b/drivers/infiniband/sw/rxe/rxe.h
index ecdba2f..1ac5b85 100644
--- a/drivers/infiniband/sw/rxe/rxe.h
+++ b/drivers/infiniband/sw/rxe/rxe.h
@@ -68,6 +68,7 @@
static inline u32 rxe_crc32(struct rxe_dev *rxe,
u32 crc, void *next, size_t len)
{
+ u32 retval;
int err;
SHASH_DESC_ON_STACK(shash, rxe->tfm);
@@ -81,7 +82,9 @@ static inline u32 rxe_crc32(struct rxe_dev *rxe,
return crc32_le(crc, next, len);
}
- return *(u32 *)shash_desc_ctx(shash);
+ retval = *(u32 *)shash_desc_ctx(shash);
+ barrier_data(shash_desc_ctx(shash));
+ return retval;
}
int rxe_set_mtu(struct rxe_dev *rxe, unsigned int dev_mtu);
diff --git a/fs/btrfs/hash.c b/fs/btrfs/hash.c
index a97fdc1..baacc18 100644
--- a/fs/btrfs/hash.c
+++ b/fs/btrfs/hash.c
@@ -38,6 +38,7 @@ u32 btrfs_crc32c(u32 crc, const void *address, unsigned int
length)
{
SHASH_DESC_ON_STACK(shash, tfm);
u32 *ctx = (u32 *)shash_desc_ctx(shash);
+ u32 retval;
int err;
shash->tfm = tfm;
@@ -47,5 +48,7 @@ u32 btrfs_crc32c(u32 crc, const void *address, unsigned int
length)
err = crypto_shash_update(shash, address, length);
BUG_ON(err);
- return *ctx;
+ retval = *ctx;
+ barrier_data(ctx);
+ return retval;
}
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 2185c7a..fd2e651 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1078,6 +1078,7 @@ static inline u32 f2fs_crc32(struct f2fs_sb_info *sbi,
const void *address,
{
SHASH_DESC_ON_STACK(shash, sbi->s_chksum_driver);
u32 *ctx = (u32 *)shash_desc_ctx(shash);
+ u32 retval;
int err;
shash->tfm = sbi->s_chksum_driver;
@@ -1087,7 +1088,9 @@ static inline u32 f2fs_crc32(struct f2fs_sb_info *sbi,
const void *address,
err = crypto_shash_update(shash, address, length);
BUG_ON(err);
- return *ctx;
+ retval = *ctx;
+ barrier_data(ctx);
+ return retval;
}
static inline bool f2fs_crc_valid(struct f2fs_sb_info *sbi, __u32 blk_crc,
diff --git a/lib/libcrc32c.c b/lib/libcrc32c.c
index 74a54b7..9f79547 100644
--- a/lib/libcrc32c.c
+++ b/lib/libcrc32c.c
@@ -43,7 +43,7 @@ static struct crypto_shash *tfm;
u32 crc32c(u32 crc, const void *address, unsigned int length)
{
SHASH_DESC_ON_STACK(shash, tfm);
- u32 *ctx = (u32 *)shash_desc_ctx(shash);
+ u32 ret, *ctx = (u32 *)shash_desc_ctx(shash);
int err;
shash->tfm = tfm;
@@ -53,7 +53,9 @@ u32 crc32c(u32 crc, const void *address, unsigned int length)
err = crypto_shash_update(shash, address, length);
BUG_ON(err);
- return *ctx;
+ ret = *ctx;
+ barrier_data(ctx);
+ return ret;
}
EXPORT_SYMBOL(crc32c);
--
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 when mounting btrfs partition, kernel unaligned access
From: David Sterba
Date: Wed, 13 Apr 2011 11:40:37 +0200
> On Wed, Apr 13, 2011 at 01:03:56AM +0200, Sébastien Bernard wrote:
>> Then, after writing on the disk, I got a lot of warning:
>> [ 822.515875] Kernel unaligned access at TPC[103c2204]
>>
>> I peeked a look at the btrf_csum_final and here's the function :
>> void btrfs_csum_final(u32 crc, char *result)
>> {
>> *(__le32 *)result = ~cpu_to_le32(crc);
>> }
>
> FYI, this has been fixed and is already merged into Linus' tree. Commit
> 7e75bf3ff3a716d7b21d8fb43bf823115801c1e9.
Might I suggest adding a BUG_ON() validation of the alignment or
similar here?
You can make the test really cheap, and this way no matter what kind
of systems the btrfs folks do their testing on this kind of regression
will be spotted fast.
--
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: Kernel unaligned access at TPC[10101f18] btrfs_csum_final+0x38/0x60
From: Christian Kujau Date: Fri, 5 Feb 2010 22:28:47 -0800 (PST) > Hm, now it looks like this, but I don't know how it'd reveal more > information: > > [ 210.707051] Kernel unaligned access at TPC[10101f18] > btrfs_csum_final+0x38/0x60 [btrfs] > [ 210.802236] Caller [10101f1c:btrfs_csum_final+0x3c/0x60 [btrfs]] > [ 210.874620] Kernel unaligned access at TPC[10101f18] > btrfs_csum_final+0x38/0x60 [btrfs] > [ 210.969899] Caller [10101f1c:btrfs_csum_final+0x3c/0x60 [btrfs]] > [ 228.724982] Kernel unaligned access at TPC[10101f18] > btrfs_csum_final+0x38/0x60 [btrfs] > [ 228.820220] Caller [10101f1c:btrfs_csum_final+0x3c/0x60 [btrfs]] > [ 228.892286] Kernel unaligned access at TPC[10101f18] > btrfs_csum_final+0x38/0x60 [btrfs] > [ 228.987851] Caller [10101f1c:btrfs_csum_final+0x3c/0x60 [btrfs]] > > I'm open for more patches :-) Confusing for me too. I'll try to think about this some more. -- 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: Kernel unaligned access at TPC[10101f18] btrfs_csum_final+0x38/0x60
From: Christian Kujau
Date: Fri, 5 Feb 2010 21:13:00 -0800 (PST)
> On Fri, 5 Feb 2010 at 12:01, David Miller wrote:
>> Can you rerun your test with the following patch applied?
>> It will obtain more information for the btrfs developers.
>
> Thanks, David! Here it is:
>
> [ 1861.965178] Kernel unaligned access at TPC[10101f18]
> btrfs_csum_final+0x38/0x60 [btrfs]
> [ 1862.060546] Caller [100a6044:crc32c+0x44/0x80 [libcrc32c]]
> [ 1862.126652] Kernel unaligned access at TPC[10101f18]
> btrfs_csum_final+0x38/0x60 [btrfs]
> [ 1862.221928] Caller [100a6044:crc32c+0x44/0x80 [libcrc32c]]
>
> It's always libcrc32c and the numbers stay the same too. Full dmesg here:
> http://nerdbynature.de/bits/2.6.33-rc6/btrfs/
My debugging patch didn't work correctly.
Can you try using this one instead?
Thanks!
diff --git a/arch/sparc/kernel/unaligned_64.c b/arch/sparc/kernel/unaligned_64.c
index 378ca82..cbde2ea 100644
--- a/arch/sparc/kernel/unaligned_64.c
+++ b/arch/sparc/kernel/unaligned_64.c
@@ -283,6 +283,9 @@ static void log_unaligned(struct pt_regs *regs)
count++;
printk("Kernel unaligned access at TPC[%lx] %pS\n",
regs->tpc, (void *) regs->tpc);
+ printk("Caller [%lx:%pS]\n",
+ regs->tnpc,
+ (void *) regs->tnpc);
}
}
--
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: Kernel unaligned access at TPC[10101f18] btrfs_csum_final+0x38/0x60
From: Christian Kujau
Date: Fri, 5 Feb 2010 10:36:52 -0800 (PST)
> When writing to a newly created btrfs (vanilla 2.6.33-rc6, sparc64) the
> following messages are printed:
>
> [28617.650231] Kernel unaligned access at TPC[10101f18]
> btrfs_csum_final+0x38/0x60 [btrfs]
> [28617.745783] Kernel unaligned access at TPC[10101f18]
> btrfs_csum_final+0x38/0x60 [btrfs]
> [28654.589492] Kernel unaligned access at TPC[10101f18]
> btrfs_csum_final+0x38/0x60 [btrfs]
> [28654.685036] Kernel unaligned access at TPC[10101f18]
> btrfs_csum_final+0x38/0x60 [btrfs]
> [28689.595394] Kernel unaligned access at TPC[10101f18]
> btrfs_csum_final+0x38/0x60 [btrfs]
Can you rerun your test with the following patch applied?
It will obtain more information for the btrfs developers.
Thanks!
diff --git a/arch/sparc/kernel/unaligned_64.c b/arch/sparc/kernel/unaligned_64.c
index 378ca82..cbde2ea 100644
--- a/arch/sparc/kernel/unaligned_64.c
+++ b/arch/sparc/kernel/unaligned_64.c
@@ -283,6 +283,9 @@ static void log_unaligned(struct pt_regs *regs)
count++;
printk("Kernel unaligned access at TPC[%lx] %pS\n",
regs->tpc, (void *) regs->tpc);
+ printk("Caller [%lx:%pS]\n",
+ regs->u_regs[UREG_RETPC],
+ (void *) regs->u_regs[UREG_RETPC]);
}
}
--
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: [PATCH -v7][RFC]: mutex: implement adaptive spinning
From: Linus Torvalds Date: Thu, 8 Jan 2009 19:46:30 -0800 (PST) > First off, gcc _does_ have a perfectly fine notion of how heavy-weight an > "asm" statement is: just count it as a single instruction (and count the > argument setup cost that gcc _can_ estimate). Actually, at least at one point, it counted the number of newline characters in the assembly string to estimate how many instructions are contained inside. It actually needs to know exaclty how many instructions are in there, to emit proper far branches and stuff like that, for some cpus. Since they never added an (optional) way to actually tell the compiler this critical piece of information, so I guess the newline hack is the best they could come up with. -- 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
