Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1278300 > unrolled thread
| Started by | Al Viro <viro@ZenIV.linux.org.uk> |
|---|---|
| First post | 2015-11-26 16:30 +0100 |
| Last post | 2015-11-26 21:30 +0100 |
| Articles | 7 — 5 participants |
Back to article view | Back to linux.kernel
[PATCH] fix an endianness bug in ext4_encrypted_follow_link() Al Viro <viro@ZenIV.linux.org.uk> - 2015-11-26 16:30 +0100
Re: [PATCH] fix an endianness bug in ext4_encrypted_follow_link() Joe Perches <joe@perches.com> - 2015-11-26 20:50 +0100
Re: [PATCH] fix an endianness bug in ext4_encrypted_follow_link() Al Viro <viro@ZenIV.linux.org.uk> - 2015-11-26 22:10 +0100
Re: [PATCH] fix an endianness bug in ext4_encrypted_follow_link() Julia Lawall <julia.lawall@lip6.fr> - 2015-11-26 22:30 +0100
Re: [PATCH] fix an endianness bug in ext4_encrypted_follow_link() Joe Perches <joe@perches.com> - 2015-11-26 23:50 +0100
Re: [PATCH] fix an endianness bug in ext4_encrypted_follow_link() Fengguang Wu <lkp@intel.com> - 2015-11-27 02:40 +0100
Re: [PATCH] fix an endianness bug in ext4_encrypted_follow_link() Theodore Ts'o <tytso@mit.edu> - 2015-11-26 21:30 +0100
| From | Al Viro <viro@ZenIV.linux.org.uk> |
|---|---|
| Date | 2015-11-26 16:30 +0100 |
| Subject | [PATCH] fix an endianness bug in ext4_encrypted_follow_link() |
| Message-ID | <qz6EI-6XT-55@gated-at.bofh.it> |
applying le32_to_cpu() to 16bit value is a bad idea...
Cc: stable@vger.kernel.org # v4.1+
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c
index abe2401..e8e7af6 100644
--- a/fs/ext4/symlink.c
+++ b/fs/ext4/symlink.c
@@ -52,7 +52,7 @@ static const char *ext4_encrypted_follow_link(struct dentry *dentry, void **cook
/* Symlink is encrypted */
sd = (struct ext4_encrypted_symlink_data *)caddr;
cstr.name = sd->encrypted_path;
- cstr.len = le32_to_cpu(sd->len);
+ cstr.len = le16_to_cpu(sd->len);
if ((cstr.len +
sizeof(struct ext4_encrypted_symlink_data) - 1) >
max_size) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-11-26 20:50 +0100 |
| Message-ID | <qzaIh-13l-1@gated-at.bofh.it> |
| In reply to | #1278300 |
(cc'ing Julia Lawall)
On Thu, 2015-11-26 at 15:27 +0000, Al Viro wrote:
> applying le32_to_cpu() to 16bit value is a bad idea...
Julia, perhaps you or your crew could produce a coccinelle test
for this class of error?
> diff --git a/fs/ext4/symlink.c b/fs/ext4/symlink.c
> index abe2401..e8e7af6 100644
> --- a/fs/ext4/symlink.c
> +++ b/fs/ext4/symlink.c
> @@ -52,7 +52,7 @@ static const char *ext4_encrypted_follow_link(struct dentry *dentry, void **cook
> /* Symlink is encrypted */
> sd = (struct ext4_encrypted_symlink_data *)caddr;
> cstr.name = sd->encrypted_path;
> - cstr.len = le32_to_cpu(sd->len);
> + cstr.len = le16_to_cpu(sd->len);
> if ((cstr.len +
> sizeof(struct ext4_encrypted_symlink_data) - 1) >
> max_size) {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Al Viro <viro@ZenIV.linux.org.uk> |
|---|---|
| Date | 2015-11-26 22:10 +0100 |
| Message-ID | <qzbXH-22g-9@gated-at.bofh.it> |
| In reply to | #1278415 |
On Thu, Nov 26, 2015 at 11:40:37AM -0800, Joe Perches wrote:
> (cc'ing Julia Lawall)
>
> On Thu, 2015-11-26 at 15:27 +0000, Al Viro wrote:
> > applying le32_to_cpu() to 16bit value is a bad idea...
>
> Julia, perhaps you or your crew could produce a coccinelle test
> for this class of error?
What's wrong with something like make C=2 CF=-D__CHECK_ENDIAN__ fs/ext4/ ?
Worked just fine, TYVM -
CHECK fs/ext4/symlink.c
fs/ext4/symlink.c:55:21: warning: cast to restricted __le32
fs/ext4/symlink.c:55:21: warning: cast from restricted __le16
points to exact location... Sure, you need to figure out _how_ it's
broken (e.g. with something like
struct foo {
__le32 a;
__le16 b;
} *p;
le32_to_cpu(p->b) might have been misspelled le16_to_cpu(p->b) as well
as le32_to_cpu(p->a)) - no way to tell one from another without actually
reading and understanding the code in question. But that doesn't depend
upon the tool used to locate the damn thing and sparse does locate them...
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julia Lawall <julia.lawall@lip6.fr> |
|---|---|
| Date | 2015-11-26 22:30 +0100 |
| Message-ID | <qzch5-29F-65@gated-at.bofh.it> |
| In reply to | #1278451 |
On Thu, 26 Nov 2015, Al Viro wrote:
> On Thu, Nov 26, 2015 at 11:40:37AM -0800, Joe Perches wrote:
> > (cc'ing Julia Lawall)
> >
> > On Thu, 2015-11-26 at 15:27 +0000, Al Viro wrote:
> > > applying le32_to_cpu() to 16bit value is a bad idea...
> >
> > Julia, perhaps you or your crew could produce a coccinelle test
> > for this class of error?
>
> What's wrong with something like make C=2 CF=-D__CHECK_ENDIAN__ fs/ext4/ ?
> Worked just fine, TYVM -
> CHECK fs/ext4/symlink.c
> fs/ext4/symlink.c:55:21: warning: cast to restricted __le32
> fs/ext4/symlink.c:55:21: warning: cast from restricted __le16
> points to exact location... Sure, you need to figure out _how_ it's
> broken (e.g. with something like
> struct foo {
> __le32 a;
> __le16 b;
> } *p;
> le32_to_cpu(p->b) might have been misspelled le16_to_cpu(p->b) as well
> as le32_to_cpu(p->a)) - no way to tell one from another without actually
> reading and understanding the code in question. But that doesn't depend
> upon the tool used to locate the damn thing and sparse does locate them...
As long as the code of interest is getting compiled in the current
configuration, relying on the compiler fo this seems like a better choice.
Coccinelle has no idea what types represent 16 values, and if one were to
try to enumerate them one would surely miss something.
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2015-11-26 23:50 +0100 |
| Message-ID | <qzdwt-2Qq-9@gated-at.bofh.it> |
| In reply to | #1278457 |
On Thu, 2015-11-26 at 22:28 +0100, Julia Lawall wrote: > On Thu, 26 Nov 2015, Al Viro wrote: > > On Thu, Nov 26, 2015 at 11:40:37AM -0800, Joe Perches wrote: > > (cc'ing Julia Lawall) > > > On Thu, 2015-11-26 at 15:27 +0000, Al Viro wrote: > > > applying le32_to_cpu() to 16bit value is a bad idea... > > Julia, perhaps you or your crew could produce a coccinelle test > > for this class of error? > What's wrong with something like make C=2 CF=-D__CHECK_ENDIAN__ fs/ext4/ ? > Worked just fine, TYVM - > sparse does locate them... Nothing at all. > As long as the code of interest is getting compiled in the current > configuration, relying on the compiler for this seems like a better choice. Sparse isn't the compiler, but that would be fine by me as long as something can catch them. The original commit (f348c252320b9) was from April. Isn't the kbuild robot using sparse and __CHECK_ENDIAN__? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Fengguang Wu <lkp@intel.com> |
|---|---|
| Date | 2015-11-27 02:40 +0100 |
| Message-ID | <qzgaZ-4xi-1@gated-at.bofh.it> |
| In reply to | #1278478 |
On Thu, Nov 26, 2015 at 02:47:56PM -0800, Joe Perches wrote:
> On Thu, 2015-11-26 at 22:28 +0100, Julia Lawall wrote:
> > On Thu, 26 Nov 2015, Al Viro wrote:
> > > On Thu, Nov 26, 2015 at 11:40:37AM -0800, Joe Perches wrote:
> > > (cc'ing Julia Lawall)
> > > > On Thu, 2015-11-26 at 15:27 +0000, Al Viro wrote:
> > > > applying le32_to_cpu() to 16bit value is a bad idea...
> > > Julia, perhaps you or your crew could produce a coccinelle test
> > > for this class of error?
> > What's wrong with something like make C=2 CF=-D__CHECK_ENDIAN__ fs/ext4/ ?
> > Worked just fine, TYVM -
> > sparse does locate them...
>
> Nothing at all.
>
> > As long as the code of interest is getting compiled in the current
> > configuration, relying on the compiler for this seems like a better choice.
>
> Sparse isn't the compiler, but that would be fine by me
> as long as something can catch them.
>
> The original commit (f348c252320b9) was from April.
> Isn't the kbuild robot using sparse and __CHECK_ENDIAN__?
Yes 0day did catch the sparse warning, however it seems the email
somehow failed to get delivered. Here is the local record:
Date: Mon, 13 Apr 2015 16:41:54 +0800
From: kbuild test robot <fengguang.wu@intel.com>
To: Theodore Ts'o <tytso@mit.edu>
Cc: kbuild-all@01.org, Uday Savagaonkar <savagaon@google.com>
Subject: [ext4:dev 32/33] fs/ext4/namei.c:3262:25: sparse: incorrect type in
assignment (different base types)
Message-ID: <201504131652.Ox8dW5C0%fengguang.wu@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.5.23 (2014-03-12)
tree: git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4.git dev
head: 3a19824f63e0a0df99c0a133097eb87c0152545e
commit: f1195c72c95115858123813e9a84badad37424c0 [32/33] ext4 crypto: Add symlink encryption
reproduce:
# apt-get install sparse
git checkout f1195c72c95115858123813e9a84badad37424c0
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
>> fs/ext4/namei.c:3262:25: sparse: incorrect type in assignment (different base types)
fs/ext4/namei.c:3262:25: expected restricted __le16 [usertype] len
fs/ext4/namei.c:3262:25: got restricted __le32 [usertype] <noident>
--
>> fs/ext4/symlink.c:74:29: sparse: cast to restricted __le32
>> fs/ext4/symlink.c:74:29: sparse: cast from restricted __le16
Thanks,
Fengguang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Theodore Ts'o <tytso@mit.edu> |
|---|---|
| Date | 2015-11-26 21:30 +0100 |
| Message-ID | <qzbl0-1y5-7@gated-at.bofh.it> |
| In reply to | #1278300 |
On Thu, Nov 26, 2015 at 03:27:28PM +0000, Al Viro wrote: > applying le32_to_cpu() to 16bit value is a bad idea... > > Cc: stable@vger.kernel.org # v4.1+ > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk> Thanks, applied. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web