[PATCH] sysfs: return correct error code on unimplemented mmap()

[Vladimir Zapolskiy]

Both POSIX.1-2008 and Linux Programmer's Manual have a dedicated return
error code for a case, when a file doesn't support mmap(), it's ENODEV.

This change replaces overloaded EINVAL with ENODEV in a situation
described above for sysfs binary files.

Signed-off-by: Vladimir Zapolskiy <vladimir_...@mentor.com>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
---
fs/sysfs/bin.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/fs/sysfs/bin.c b/fs/sysfs/bin.c
index c590cab..d37433c 100644
--- a/fs/sysfs/bin.c
+++ b/fs/sysfs/bin.c
@@ -350,7 +350,6 @@ static int mmap(struct file *file, struct vm_area_struct *vma)
if (!sysfs_get_active(attr_sd))
goto out_unlock;

- rc = -EINVAL;
if (!attr->mmap)
goto out_put;

--
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Greg Kroah-Hartman]

On Mon, Oct 28, 2013 at 06:28:30PM +0200, Vladimir Zapolskiy wrote:
> Both POSIX.1-2008 and Linux Programmer's Manual have a dedicated return
> error code for a case, when a file doesn't support mmap(), it's ENODEV.
>
> This change replaces overloaded EINVAL with ENODEV in a situation
> described above for sysfs binary files.
>
> Signed-off-by: Vladimir Zapolskiy <vladimir_...@mentor.com>
> Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>

Is this a problem in linux-next or 3.12-rc7 and older kernels? What
userspace tool is breaking here?

thanks,

greg k-h

[Vladimir Zapolskiy]

On 10/28/13 18:35, Greg Kroah-Hartman wrote:
> On Mon, Oct 28, 2013 at 06:28:30PM +0200, Vladimir Zapolskiy wrote:
>> Both POSIX.1-2008 and Linux Programmer's Manual have a dedicated return
>> error code for a case, when a file doesn't support mmap(), it's ENODEV.
>>
>> This change replaces overloaded EINVAL with ENODEV in a situation
>> described above for sysfs binary files.
>>
>> Signed-off-by: Vladimir Zapolskiy<vladimir_...@mentor.com>
>> Cc: Greg Kroah-Hartman<gre...@linuxfoundation.org>
>
> Is this a problem in linux-next or 3.12-rc7 and older kernels? What
> userspace tool is breaking here?

The patch itself is against 3.12-rc7, but the same problem can be found
in older kernels as well.

I assume the bugfix is quite safe, because both EINVAL and ENODEV should
be handled by user space tools, however for developers it might be
beneficial to distinguish cases of completely not supported mmap() and
supported but misused mmap() applied to a binary sysfs file. At the
moment from the user space perspective there is no indication of
unsupported mmap() over a file in sysfs.

With best wishes,
Vladimir

[Greg Kroah-Hartman]

On Mon, Oct 28, 2013 at 07:22:47PM +0200, Vladimir Zapolskiy wrote:
> On 10/28/13 18:35, Greg Kroah-Hartman wrote:
> >On Mon, Oct 28, 2013 at 06:28:30PM +0200, Vladimir Zapolskiy wrote:
> >>Both POSIX.1-2008 and Linux Programmer's Manual have a dedicated return
> >>error code for a case, when a file doesn't support mmap(), it's ENODEV.
> >>
> >>This change replaces overloaded EINVAL with ENODEV in a situation
> >>described above for sysfs binary files.
> >>
> >>Signed-off-by: Vladimir Zapolskiy<vladimir_...@mentor.com>
> >>Cc: Greg Kroah-Hartman<gre...@linuxfoundation.org>
> >
> >Is this a problem in linux-next or 3.12-rc7 and older kernels? What
> >userspace tool is breaking here?
>
> The patch itself is against 3.12-rc7, but the same problem can be found
> in older kernels as well.
>
> I assume the bugfix is quite safe, because both EINVAL and ENODEV should
> be handled by user space tools, however for developers it might be
> beneficial to distinguish cases of completely not supported mmap() and
> supported but misused mmap() applied to a binary sysfs file. At the
> moment from the user space perspective there is no indication of
> unsupported mmap() over a file in sysfs.

Ok, fair enough, but it doesn't apply to my tree given the recent sysfs
changes there. Can you redo this against linux-next, or wait for
3.13-rc1 and resend it against that release in a few weeks?

thanks,

greg k-h

[Vladimir Zapolskiy]

Both POSIX.1-2008 and Linux Programmer's Manual have a dedicated return
error code for a case, when a file doesn't support mmap(), it's ENODEV.

This change replaces overloaded EINVAL with ENODEV in a situation
described above for sysfs binary files.

Signed-off-by: Vladimir Zapolskiy <vladimir_...@mentor.com>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>

---
fs/sysfs/file.c | 1 -

1 file changed, 1 deletion(-)

diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
index 0d7368d4..382db3c 100644
--- a/fs/sysfs/file.c
+++ b/fs/sysfs/file.c
@@ -477,7 +477,6 @@ static int sysfs_bin_mmap(struct file *file, struct vm_area_struct *vma)
if (!sysfs_get_active(of->sd))

goto out_unlock;

- rc = -EINVAL;

if (!battr->mmap)
goto out_put;

--
1.7.10.4

[Vladimir Zapolskiy]

Greg,

On 10/30/13 14:08, Vladimir Zapolskiy wrote:
> Both POSIX.1-2008 and Linux Programmer's Manual have a dedicated return
> error code for a case, when a file doesn't support mmap(), it's ENODEV.
>
> This change replaces overloaded EINVAL with ENODEV in a situation
> described above for sysfs binary files.
>
> Signed-off-by: Vladimir Zapolskiy<vladimir_...@mentor.com>
> Cc: Greg Kroah-Hartman<gre...@linuxfoundation.org>
> ---
> fs/sysfs/file.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c
> index 0d7368d4..382db3c 100644
> --- a/fs/sysfs/file.c
> +++ b/fs/sysfs/file.c
> @@ -477,7 +477,6 @@ static int sysfs_bin_mmap(struct file *file, struct vm_area_struct *vma)
> if (!sysfs_get_active(of->sd))
> goto out_unlock;
>
> - rc = -EINVAL;
> if (!battr->mmap)
> goto out_put;
>

as you asked the patch is rebased on top of linux-next, if you encounter
any problems related to the change, please let me know.

Thanks,
Vladimir