* [PATCH] ecryptfs: Fix explicit null dereference
@ 2013-11-14 18:42 Geyslan G. Bem
2013-11-14 18:50 ` Pekka Enberg
2013-11-14 19:52 ` Tyler Hicks
0 siblings, 2 replies; 6+ messages in thread
From: Geyslan G. Bem @ 2013-11-14 18:42 UTC (permalink / raw)
To: geyslan; +Cc: Tyler Hicks, open list:ECRYPT FILE SYSTEM, open list
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 <geyslan@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
--
1.8.4.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] ecryptfs: Fix explicit null dereference
2013-11-14 18:42 [PATCH] ecryptfs: Fix explicit null dereference Geyslan G. Bem
@ 2013-11-14 18:50 ` Pekka Enberg
2013-11-14 19:52 ` Tyler Hicks
1 sibling, 0 replies; 6+ messages in thread
From: Pekka Enberg @ 2013-11-14 18:50 UTC (permalink / raw)
To: Geyslan G. Bem; +Cc: Tyler Hicks, open list:ECRYPT FILE SYSTEM, open list
On Thu, Nov 14, 2013 at 8:42 PM, Geyslan G. Bem <geyslan@gmail.com> wrote:
> 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 <geyslan@gmail.com>
Reviewed-by: Pekka Enberg <penberg@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ecryptfs: Fix explicit null dereference
2013-11-14 18:42 [PATCH] ecryptfs: Fix explicit null dereference Geyslan G. Bem
2013-11-14 18:50 ` Pekka Enberg
@ 2013-11-14 19:52 ` Tyler Hicks
2013-11-14 19:58 ` Geyslan Gregório Bem
1 sibling, 1 reply; 6+ messages in thread
From: Tyler Hicks @ 2013-11-14 19:52 UTC (permalink / raw)
To: Geyslan G. Bem; +Cc: ecryptfs, linux-kernel, Dan Carpenter, Pekka Enberg
[-- Attachment #1: Type: text/plain, Size: 2177 bytes --]
On 2013-11-14 15:42:14, Geyslan G. Bem wrote:
> 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 <geyslan@gmail.com>
> ---
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
> 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
> --
> 1.8.4.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ecryptfs: Fix explicit null dereference
2013-11-14 19:52 ` Tyler Hicks
@ 2013-11-14 19:58 ` Geyslan Gregório Bem
2013-11-14 20:02 ` Tyler Hicks
0 siblings, 1 reply; 6+ messages in thread
From: Geyslan Gregório Bem @ 2013-11-14 19:58 UTC (permalink / raw)
To: Tyler Hicks; +Cc: ecryptfs, LKML, Dan Carpenter, Pekka Enberg
2013/11/14 Tyler Hicks <tyhicks@canonical.com>:
> On 2013-11-14 15:42:14, Geyslan G. Bem wrote:
>> 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 <geyslan@gmail.com>
>> ---
>
> 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
True. Disregard mine.
Thanks Tyler.
>
>> 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
>> --
>> 1.8.4.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Regards,
Geyslan G. Bem
hackingbits.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ecryptfs: Fix explicit null dereference
2013-11-14 19:58 ` Geyslan Gregório Bem
@ 2013-11-14 20:02 ` Tyler Hicks
2013-11-14 20:04 ` Geyslan Gregório Bem
0 siblings, 1 reply; 6+ messages in thread
From: Tyler Hicks @ 2013-11-14 20:02 UTC (permalink / raw)
To: Geyslan Gregório Bem; +Cc: ecryptfs, LKML, Dan Carpenter, Pekka Enberg
[-- Attachment #1: Type: text/plain, Size: 3039 bytes --]
On 2013-11-14 17:58:40, Geyslan Gregório Bem wrote:
> 2013/11/14 Tyler Hicks <tyhicks@canonical.com>:
> > On 2013-11-14 15:42:14, Geyslan G. Bem wrote:
> >> 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 <geyslan@gmail.com>
> >> ---
> >
> > 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
>
> 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
>
> >
> >> 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
> >> --
> >> 1.8.4.2
> >>
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
> >> the body of a message to majordomo@vger.kernel.org
> >> 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
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ecryptfs: Fix explicit null dereference
2013-11-14 20:02 ` Tyler Hicks
@ 2013-11-14 20:04 ` Geyslan Gregório Bem
0 siblings, 0 replies; 6+ messages in thread
From: Geyslan Gregório Bem @ 2013-11-14 20:04 UTC (permalink / raw)
To: Tyler Hicks
Cc: open list:ECRYPT FILE SYSTEM, LKML, Dan Carpenter, Pekka Enberg
2013/11/14 Tyler Hicks <tyhicks@canonical.com>:
> On 2013-11-14 17:58:40, Geyslan Gregório Bem wrote:
>> 2013/11/14 Tyler Hicks <tyhicks@canonical.com>:
>> > On 2013-11-14 15:42:14, Geyslan G. Bem wrote:
>> >> 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 <geyslan@gmail.com>
>> >> ---
>> >
>> > 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
>>
>> 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.
>>
>> >
>> >> 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
>> >> --
>> >> 1.8.4.2
>> >>
>> >> --
>> >> To unsubscribe from this list: send the line "unsubscribe ecryptfs" in
>> >> the body of a message to majordomo@vger.kernel.org
>> >> 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
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Regards,
Geyslan G. Bem
hackingbits.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-14 20:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-14 18:42 [PATCH] ecryptfs: Fix explicit null dereference Geyslan G. Bem
2013-11-14 18:50 ` Pekka Enberg
2013-11-14 19:52 ` Tyler Hicks
2013-11-14 19:58 ` Geyslan Gregório Bem
2013-11-14 20:02 ` Tyler Hicks
2013-11-14 20:04 ` Geyslan Gregório Bem
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox