When the call to nfs_devname() fails, the error path attempts to retain
the error via the mnt variable, but this requires a cast across very
different types (char * to struct vfsmount *), which the upcoming
structure layout randomization plugin flags as being potentially
dangerous in the face of randomization. This is a false positive, but
what this code actually wants to do is retain the error value, so this
patch explicitly sets it, instead of using what seems to be an
unexpected cast.
Signed-off-by: Kees Cook <[email protected]>
---
v2: duh, use ERR_CAST. thanks neilb!
---
fs/nfs/namespace.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
index 786f17580582..8ca5d147124d 100644
--- a/fs/nfs/namespace.c
+++ b/fs/nfs/namespace.c
@@ -259,9 +259,10 @@ struct vfsmount *nfs_do_submount(struct dentry *dentry, struct nfs_fh *fh,
if (page == NULL)
goto out;
devname = nfs_devname(dentry, page, PAGE_SIZE);
- mnt = (struct vfsmount *)devname;
- if (IS_ERR(devname))
+ if (IS_ERR(devname)) {
+ mnt = ERR_CAST(devname);
goto free_page;
+ }
mnt = nfs_do_clone_mount(NFS_SB(dentry->d_sb), devname, &mountdata);
free_page:
free_page((unsigned long)page);
--
2.7.4
--
Kees Cook
Pixel Security
On Wed, Apr 5, 2017 at 8:29 AM, Kees Cook <[email protected]> wrote:
> When the call to nfs_devname() fails, the error path attempts to retain
> the error via the mnt variable, but this requires a cast across very
> different types (char * to struct vfsmount *), which the upcoming
> structure layout randomization plugin flags as being potentially
> dangerous in the face of randomization. This is a false positive, but
> what this code actually wants to do is retain the error value, so this
> patch explicitly sets it, instead of using what seems to be an
> unexpected cast.
>
> Signed-off-by: Kees Cook <[email protected]>
If I can get an Acked-by on this, I could push it via the gcc-plugin tree.
Thanks!
-Kees
> ---
> v2: duh, use ERR_CAST. thanks neilb!
> ---
> fs/nfs/namespace.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nfs/namespace.c b/fs/nfs/namespace.c
> index 786f17580582..8ca5d147124d 100644
> --- a/fs/nfs/namespace.c
> +++ b/fs/nfs/namespace.c
> @@ -259,9 +259,10 @@ struct vfsmount *nfs_do_submount(struct dentry *dentry, struct nfs_fh *fh,
> if (page == NULL)
> goto out;
> devname = nfs_devname(dentry, page, PAGE_SIZE);
> - mnt = (struct vfsmount *)devname;
> - if (IS_ERR(devname))
> + if (IS_ERR(devname)) {
> + mnt = ERR_CAST(devname);
> goto free_page;
> + }
> mnt = nfs_do_clone_mount(NFS_SB(dentry->d_sb), devname, &mountdata);
> free_page:
> free_page((unsigned long)page);
> --
> 2.7.4
>
>
> --
> Kees Cook
> Pixel Security
--
Kees Cook
Pixel Security
T24gV2VkLCAyMDE3LTA0LTEyIGF0IDExOjU5IC0wNzAwLCBLZWVzIENvb2sgd3JvdGU6DQo+IE9u
IFdlZCwgQXByIDUsIDIwMTcgYXQgODoyOSBBTSwgS2VlcyBDb29rIDxrZWVzY29va0BjaHJvbWl1
bS5vcmc+DQo+IHdyb3RlOg0KPiA+IFdoZW4gdGhlIGNhbGwgdG8gbmZzX2Rldm5hbWUoKSBmYWls
cywgdGhlIGVycm9yIHBhdGggYXR0ZW1wdHMgdG8NCj4gPiByZXRhaW4NCj4gPiB0aGUgZXJyb3Ig
dmlhIHRoZSBtbnQgdmFyaWFibGUsIGJ1dCB0aGlzIHJlcXVpcmVzIGEgY2FzdCBhY3Jvc3MNCj4g
PiB2ZXJ5DQo+ID4gZGlmZmVyZW50IHR5cGVzIChjaGFyICogdG8gc3RydWN0IHZmc21vdW50ICop
LCB3aGljaCB0aGUgdXBjb21pbmcNCj4gPiBzdHJ1Y3R1cmUgbGF5b3V0IHJhbmRvbWl6YXRpb24g
cGx1Z2luIGZsYWdzIGFzIGJlaW5nIHBvdGVudGlhbGx5DQo+ID4gZGFuZ2Vyb3VzIGluIHRoZSBm
YWNlIG9mIHJhbmRvbWl6YXRpb24uIFRoaXMgaXMgYSBmYWxzZSBwb3NpdGl2ZSwNCj4gPiBidXQN
Cj4gPiB3aGF0IHRoaXMgY29kZSBhY3R1YWxseSB3YW50cyB0byBkbyBpcyByZXRhaW4gdGhlIGVy
cm9yIHZhbHVlLCBzbw0KPiA+IHRoaXMNCj4gPiBwYXRjaCBleHBsaWNpdGx5IHNldHMgaXQsIGlu
c3RlYWQgb2YgdXNpbmcgd2hhdCBzZWVtcyB0byBiZSBhbg0KPiA+IHVuZXhwZWN0ZWQgY2FzdC4N
Cj4gPiANCj4gPiBTaWduZWQtb2ZmLWJ5OiBLZWVzIENvb2sgPGtlZXNjb29rQGNocm9taXVtLm9y
Zz4NCj4gDQo+IElmIEkgY2FuIGdldCBhbiBBY2tlZC1ieSBvbiB0aGlzLCBJIGNvdWxkIHB1c2gg
aXQgdmlhIHRoZSBnY2MtcGx1Z2luDQo+IHRyZWUuDQo+IA0KPiBUaGFua3MhDQo+IA0KPiAtS2Vl
cw0KPiANCj4gPiAtLS0NCj4gPiB2MjogZHVoLCB1c2UgRVJSX0NBU1QuIHRoYW5rcyBuZWlsYiEN
Cj4gPiAtLS0NCj4gPiDCoGZzL25mcy9uYW1lc3BhY2UuYyB8IDUgKysrLS0NCj4gPiDCoDEgZmls
ZSBjaGFuZ2VkLCAzIGluc2VydGlvbnMoKyksIDIgZGVsZXRpb25zKC0pDQo+ID4gDQo+ID4gZGlm
ZiAtLWdpdCBhL2ZzL25mcy9uYW1lc3BhY2UuYyBiL2ZzL25mcy9uYW1lc3BhY2UuYw0KPiA+IGlu
ZGV4IDc4NmYxNzU4MDU4Mi4uOGNhNWQxNDcxMjRkIDEwMDY0NA0KPiA+IC0tLSBhL2ZzL25mcy9u
YW1lc3BhY2UuYw0KPiA+ICsrKyBiL2ZzL25mcy9uYW1lc3BhY2UuYw0KPiA+IEBAIC0yNTksOSAr
MjU5LDEwIEBAIHN0cnVjdCB2ZnNtb3VudCAqbmZzX2RvX3N1Ym1vdW50KHN0cnVjdCBkZW50cnkN
Cj4gPiAqZGVudHJ5LCBzdHJ1Y3QgbmZzX2ZoICpmaCwNCj4gPiDCoMKgwqDCoMKgwqDCoMKgaWYg
KHBhZ2UgPT0gTlVMTCkNCj4gPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGdvdG8g
b3V0Ow0KPiA+IMKgwqDCoMKgwqDCoMKgwqBkZXZuYW1lID0gbmZzX2Rldm5hbWUoZGVudHJ5LCBw
YWdlLCBQQUdFX1NJWkUpOw0KPiA+IC3CoMKgwqDCoMKgwqDCoG1udCA9IChzdHJ1Y3QgdmZzbW91
bnQgKilkZXZuYW1lOw0KPiA+IC3CoMKgwqDCoMKgwqDCoGlmIChJU19FUlIoZGV2bmFtZSkpDQo+
ID4gK8KgwqDCoMKgwqDCoMKgaWYgKElTX0VSUihkZXZuYW1lKSkgew0KPiA+ICvCoMKgwqDCoMKg
wqDCoMKgwqDCoMKgwqDCoMKgwqBtbnQgPSBFUlJfQ0FTVChkZXZuYW1lKTsNCj4gPiDCoMKgwqDC
oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGdvdG8gZnJlZV9wYWdlOw0KPiA+ICvCoMKgwqDCoMKg
wqDCoH0NCj4gPiDCoMKgwqDCoMKgwqDCoMKgbW50ID0gbmZzX2RvX2Nsb25lX21vdW50KE5GU19T
QihkZW50cnktPmRfc2IpLCBkZXZuYW1lLA0KPiA+ICZtb3VudGRhdGEpOw0KPiA+IMKgZnJlZV9w
YWdlOg0KPiA+IMKgwqDCoMKgwqDCoMKgwqBmcmVlX3BhZ2UoKHVuc2lnbmVkIGxvbmcpcGFnZSk7
DQo+ID4gLS0NCj4gPiAyLjcuNA0KPiA+IA0KPiA+IA0KPiA+IC0tDQo+ID4gS2VlcyBDb29rDQo+
ID4gUGl4ZWwgU2VjdXJpdHkNCj4gDQoNCkFja2VkLWJ5OiBUcm9uZCBNeWtsZWJ1c3QgPHRyb25k
Lm15a2xlYnVzdEBwcmltYXJ5ZGF0YS5jb20+DQoNCi0tIA0KVHJvbmQgTXlrbGVidXN0DQpMaW51
eCBORlMgY2xpZW50IG1haW50YWluZXIsIFByaW1hcnlEYXRhDQp0cm9uZC5teWtsZWJ1c3RAcHJp
bWFyeWRhdGEuY29tDQo=