[PATCH] ecryptfs: Fix explicit null dereference

[Geyslan G. Bem]

If the condition 'ecryptfs_file_to_private(file)' takes false branch
lower_file is dereferenced when NULL.

Caught by Coverity: CIDs 1128834 and 1128833.

Signed-off-by: Geyslan G. Bem <***@gmail.com>
---
fs/ecryptfs/file.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 2229a74..1c0403a 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -316,10 +316,12 @@ ecryptfs_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct file *lower_file = NULL;
long rc = -ENOTTY;

- if (ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
+ if (!ecryptfs_file_to_private(file))
+ goto out;
+ lower_file = ecryptfs_file_to_lower(file);
if (lower_file->f_op->unlocked_ioctl)
rc = lower_file->f_op->unlocked_ioctl(lower_file, cmd, arg);
+out:
return rc;
}

@@ -330,10 +332,12 @@ ecryptfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct file *lower_file = NULL;
long rc = -ENOIOCTLCMD;

- if (ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
+ if (!ecryptfs_file_to_private(file))
+ goto out;
+ lower_file = ecryptfs_file_to_lower(file);
if (lower_file->f_op && lower_file->f_op->compat_ioctl)
rc = lower_file->f_op->compat_ioctl(lower_file, cmd, arg);
+out:
return rc;
}
#endif

[Pekka Enberg]

Post by Geyslan G. Bem
If the condition 'ecryptfs_file_to_private(file)' takes false branch
lower_file is dereferenced when NULL.
Caught by Coverity: CIDs 1128834 and 1128833.

Reviewed-by: Pekka Enberg <***@kernel.org>

[Tyler Hicks]

Post by Geyslan G. Bem
If the condition 'ecryptfs_file_to_private(file)' takes false branch
lower_file is dereferenced when NULL.
Caught by Coverity: CIDs 1128834 and 1128833.
---

Hello - Smatch picked up on this earlier in week and Dan analyzed the
situation here:

http://article.gmane.org/gmane.comp.file-systems.ecryptfs.general/441

I agree with his assessment and proposed the following patch:

http://article.gmane.org/gmane.comp.file-systems.ecryptfs.general/442

It makes Smatch happy and it should also make Coverity happy.

Tyler

Post by Geyslan G. Bem
fs/ecryptfs/file.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 2229a74..1c0403a 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -316,10 +316,12 @@ ecryptfs_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct file *lower_file = NULL;
long rc = -ENOTTY;
- if (ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
+ if (!ecryptfs_file_to_private(file))
+ goto out;
+ lower_file = ecryptfs_file_to_lower(file);
if (lower_file->f_op->unlocked_ioctl)
rc = lower_file->f_op->unlocked_ioctl(lower_file, cmd, arg);
return rc;
}
@@ -330,10 +332,12 @@ ecryptfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct file *lower_file = NULL;
long rc = -ENOIOCTLCMD;
- if (ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
+ if (!ecryptfs_file_to_private(file))
+ goto out;
+ lower_file = ecryptfs_file_to_lower(file);
if (lower_file->f_op && lower_file->f_op->compat_ioctl)
rc = lower_file->f_op->compat_ioctl(lower_file, cmd, arg);
return rc;
}
#endif
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Geyslan Gregório Bem]

Post by Tyler Hicks

Post by Geyslan G. Bem
If the condition 'ecryptfs_file_to_private(file)' takes false branch
lower_file is dereferenced when NULL.
Caught by Coverity: CIDs 1128834 and 1128833.
---

Hello - Smatch picked up on this earlier in week and Dan analyzed the
http://article.gmane.org/gmane.comp.file-systems.ecryptfs.general/441
http://article.gmane.org/gmane.comp.file-systems.ecryptfs.general/442
It makes Smatch happy and it should also make Coverity happy.
Tyler

True. Disregard mine.

Thanks Tyler.

Post by Tyler Hicks

Post by Geyslan G. Bem
fs/ecryptfs/file.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 2229a74..1c0403a 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -316,10 +316,12 @@ ecryptfs_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct file *lower_file = NULL;
long rc = -ENOTTY;
- if (ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
+ if (!ecryptfs_file_to_private(file))
+ goto out;
+ lower_file = ecryptfs_file_to_lower(file);
if (lower_file->f_op->unlocked_ioctl)
rc = lower_file->f_op->unlocked_ioctl(lower_file, cmd, arg);
return rc;
}
@@ -330,10 +332,12 @@ ecryptfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct file *lower_file = NULL;
long rc = -ENOIOCTLCMD;
- if (ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
+ if (!ecryptfs_file_to_private(file))
+ goto out;
+ lower_file = ecryptfs_file_to_lower(file);
if (lower_file->f_op && lower_file->f_op->compat_ioctl)
rc = lower_file->f_op->compat_ioctl(lower_file, cmd, arg);
return rc;
}
#endif
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Tyler Hicks]

Post by Geyslan Gregório Bem

Post by Tyler Hicks

Post by Geyslan G. Bem
If the condition 'ecryptfs_file_to_private(file)' takes false branch
lower_file is dereferenced when NULL.
Caught by Coverity: CIDs 1128834 and 1128833.
---

Hello - Smatch picked up on this earlier in week and Dan analyzed the
http://article.gmane.org/gmane.comp.file-systems.ecryptfs.general/441
http://article.gmane.org/gmane.comp.file-systems.ecryptfs.general/442
It makes Smatch happy and it should also make Coverity happy.
Tyler

True. Disregard mine.
Thanks Tyler.

Thank you! Can I add your Reviewed-by: tag to that patch prior to
pushing it to my next branch?

Tyler

Post by Geyslan Gregório Bem

Post by Tyler Hicks

Post by Geyslan G. Bem
fs/ecryptfs/file.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 2229a74..1c0403a 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -316,10 +316,12 @@ ecryptfs_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct file *lower_file = NULL;
long rc = -ENOTTY;
- if (ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
+ if (!ecryptfs_file_to_private(file))
+ goto out;
+ lower_file = ecryptfs_file_to_lower(file);
if (lower_file->f_op->unlocked_ioctl)
rc = lower_file->f_op->unlocked_ioctl(lower_file, cmd, arg);
return rc;
}
@@ -330,10 +332,12 @@ ecryptfs_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
struct file *lower_file = NULL;
long rc = -ENOIOCTLCMD;
- if (ecryptfs_file_to_private(file))
- lower_file = ecryptfs_file_to_lower(file);
+ if (!ecryptfs_file_to_private(file))
+ goto out;
+ lower_file = ecryptfs_file_to_lower(file);
if (lower_file->f_op && lower_file->f_op->compat_ioctl)
rc = lower_file->f_op->compat_ioctl(lower_file, cmd, arg);
return rc;
}
#endif
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Regards,
Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Geyslan Gregório Bem]

Post by Tyler Hicks

Post by Geyslan Gregório Bem

If the condition 'ecryptfs_file_to_private(file)' takes false bra=

nch

Post by Tyler Hicks

Post by Geyslan Gregório Bem

lower_file is dereferenced when NULL.
Caught by Coverity: CIDs 1128834 and 1128833.
---

Hello - Smatch picked up on this earlier in week and Dan analyzed =

the

Post by Tyler Hicks

Post by Geyslan Gregório Bem

http://article.gmane.org/gmane.comp.file-systems.ecryptfs.genera=

l/441

Post by Tyler Hicks

Post by Geyslan Gregório Bem

http://article.gmane.org/gmane.comp.file-systems.ecryptfs.genera=

l/442

Post by Tyler Hicks

Post by Geyslan Gregório Bem

It makes Smatch happy and it should also make Coverity happy.
Tyler

True. Disregard mine.
Thanks Tyler.

Thank you! Can I add your Reviewed-by: tag to that patch prior to
pushing it to my next branch?
Tyler

Sure, Tyler. You can! :)

Thanks again.

Post by Tyler Hicks

Post by Geyslan Gregório Bem

fs/ecryptfs/file.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/fs/ecryptfs/file.c b/fs/ecryptfs/file.c
index 2229a74..1c0403a 100644
--- a/fs/ecryptfs/file.c
+++ b/fs/ecryptfs/file.c
@@ -316,10 +316,12 @@ ecryptfs_unlocked_ioctl(struct file *file, =

unsigned int cmd, unsigned long arg)

Post by Tyler Hicks

Post by Geyslan Gregório Bem

struct file *lower_file =3D NULL;
long rc =3D -ENOTTY;
- if (ecryptfs_file_to_private(file))
- lower_file =3D ecryptfs_file_to_lower(file);
+ if (!ecryptfs_file_to_private(file))
+ goto out;
+ lower_file =3D ecryptfs_file_to_lower(file);
if (lower_file->f_op->unlocked_ioctl)
rc =3D lower_file->f_op->unlocked_ioctl(lower_file,=

cmd, arg);

Post by Tyler Hicks

Post by Geyslan Gregório Bem

return rc;
}
@@ -330,10 +332,12 @@ ecryptfs_compat_ioctl(struct file *file, un=

signed int cmd, unsigned long arg)

Post by Tyler Hicks

Post by Geyslan Gregório Bem

struct file *lower_file =3D NULL;
long rc =3D -ENOIOCTLCMD;
- if (ecryptfs_file_to_private(file))
- lower_file =3D ecryptfs_file_to_lower(file);
+ if (!ecryptfs_file_to_private(file))
+ goto out;
+ lower_file =3D ecryptfs_file_to_lower(file);
if (lower_file->f_op && lower_file->f_op->compat_ioctl)
rc =3D lower_file->f_op->compat_ioctl(lower_file, c=

md, arg);

Post by Tyler Hicks

Post by Geyslan Gregório Bem

return rc;
}
#endif
--
1.8.4.2
--
To unsubscribe from this list: send the line "unsubscribe ecryptf=

s" in

Post by Tyler Hicks

Post by Geyslan Gregório Bem

More majordomo info at http://vger.kernel.org/majordomo-info.htm=

l

Post by Tyler Hicks

Post by Geyslan Gregório Bem
--
Regards,
Geyslan G. Bem
hackingbits.com
--
To unsubscribe from this list: send the line "unsubscribe ecryptfs" =

in

Post by Tyler Hicks

Post by Geyslan Gregório Bem
More majordomo info at http://vger.kernel.org/majordomo-info.html

--=20
Regards,

Geyslan G. Bem
hackingbits.com