Re: [Devel] [PATCH] fs/fuse kio: deny unavailable ioctl's in kio
pls ignore this.
On 25.12.2018 18:11, Pavel Butsykin wrote:
> There are some ioctl's that were made to service the kio module, and they
> should not be available to user app.
>
> Also there are two ioctls PCS_IOC_NOCSUMONREAD and PCS_IOC_NOWRITEDELAY that
> must be handled in kio, otherwise they are meaningless. So let's return
> -EOPNOTSUPP unless we implement them in kio.
>
> #VSTOR-19332
>
> Signed-off-by: Pavel Butsykin
> ---
> fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 19 ---
> fs/fuse/kio/pcs/pcs_ioctl.h| 5 +
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> index 89866370a341..13acd2e172f3 100644
> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> @@ -1110,9 +1110,22 @@ static int pcs_kio_classify_req(struct fuse_conn *fc,
> struct fuse_req *req, bool
> case FUSE_IOCTL: {
> struct fuse_ioctl_in const *inarg = req->in.args[0].value;
>
> - if (inarg->cmd != FS_IOC_FIEMAP)
> - return 1;
> -
> + switch (inarg->cmd) {
> + case FS_IOC_FIEMAP:
> + break;
> + case PCS_IOC_NOCSUMONREAD:
> + case PCS_IOC_NOWRITEDELAY:
> + return -EOPNOTSUPP;
> + case PCS_IOC_INIT_KDIRECT:
> + case PCS_IOC_CSCONN:
> + case PCS_IOC_GETFILEINFO:
> + case PCS_IOC_KDIRECT_CLAIM:
> + case PCS_IOC_KDIRECT_RELEASE:
> + case PCS_IOC_GETMAP:
> + return -EPERM;
> + default:
> + return 1;
> + }
> break;
> }
> default:
> diff --git a/fs/fuse/kio/pcs/pcs_ioctl.h b/fs/fuse/kio/pcs/pcs_ioctl.h
> index 5cf40e35f881..d6c02ae66977 100644
> --- a/fs/fuse/kio/pcs/pcs_ioctl.h
> +++ b/fs/fuse/kio/pcs/pcs_ioctl.h
> @@ -78,6 +78,11 @@ struct pcs_ioc_csconn
> #define PCS_IOC_CS_REOPEN (PCS_IOC_CS_OPEN|PCS_IOC_CS_CLOSE)
> };
>
> +#define PCS_IOC_NOCSUMONREAD _IOW('V',3,u32)
> +# define PCS_IOC_NOCSUMONREAD_DATA 1
> +# define PCS_IOC_NOCSUMONREAD_METADATA 2
> +#define PCS_IOC_NOWRITEDELAY _IOW('V',4,u32)
> +
> #define PCS_IOC_INIT_KDIRECT_IOR('V',32, struct
> pcs_ioc_init_kdirect)
> #define PCS_IOC_CSCONN _IOR('V',33, struct pcs_ioc_csconn)
> #define PCS_IOC_GETFILEINFO _IOR('V',34, struct pcs_ioc_fileinfo)
>
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
[Devel] [PATCH] fs/fuse kio: deny unavailable ioctl's in kio
There are some ioctl's that were made to service the kio module, and they
should not be available to user app.
Also there are two ioctls PCS_IOC_NOCSUMONREAD and PCS_IOC_NOWRITEDELAY that
must be handled in kio, otherwise they are meaningless. So let's return
-EOPNOTSUPP unless we implement them in kio.
#VSTOR-19332
Signed-off-by: Pavel Butsykin
---
fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 19 ---
fs/fuse/kio/pcs/pcs_ioctl.h| 5 +
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
index 89866370a341..13acd2e172f3 100644
--- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
+++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
@@ -1110,9 +1110,22 @@ static int pcs_kio_classify_req(struct fuse_conn *fc,
struct fuse_req *req, bool
case FUSE_IOCTL: {
struct fuse_ioctl_in const *inarg = req->in.args[0].value;
- if (inarg->cmd != FS_IOC_FIEMAP)
- return 1;
-
+ switch (inarg->cmd) {
+ case FS_IOC_FIEMAP:
+ break;
+ case PCS_IOC_NOCSUMONREAD:
+ case PCS_IOC_NOWRITEDELAY:
+ return -EOPNOTSUPP;
+ case PCS_IOC_INIT_KDIRECT:
+ case PCS_IOC_CSCONN:
+ case PCS_IOC_GETFILEINFO:
+ case PCS_IOC_KDIRECT_CLAIM:
+ case PCS_IOC_KDIRECT_RELEASE:
+ case PCS_IOC_GETMAP:
+ return -EPERM;
+ default:
+ return 1;
+ }
break;
}
default:
diff --git a/fs/fuse/kio/pcs/pcs_ioctl.h b/fs/fuse/kio/pcs/pcs_ioctl.h
index 5cf40e35f881..d6c02ae66977 100644
--- a/fs/fuse/kio/pcs/pcs_ioctl.h
+++ b/fs/fuse/kio/pcs/pcs_ioctl.h
@@ -78,6 +78,11 @@ struct pcs_ioc_csconn
#define PCS_IOC_CS_REOPEN (PCS_IOC_CS_OPEN|PCS_IOC_CS_CLOSE)
};
+#define PCS_IOC_NOCSUMONREAD _IOW('V',3,u32)
+# define PCS_IOC_NOCSUMONREAD_DATA 1
+# define PCS_IOC_NOCSUMONREAD_METADATA 2
+#define PCS_IOC_NOWRITEDELAY _IOW('V',4,u32)
+
#define PCS_IOC_INIT_KDIRECT _IOR('V',32, struct pcs_ioc_init_kdirect)
#define PCS_IOC_CSCONN _IOR('V',33, struct pcs_ioc_csconn)
#define PCS_IOC_GETFILEINFO_IOR('V',34, struct pcs_ioc_fileinfo)
--
2.15.1
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] fs/fuse kio: satisfy pure FALLOC_FL_KEEP_SIZE immediately
On 25.12.2018 13:00, Pavel Butsykin wrote:
>
>
> On 25.12.2018 12:28, Kirill Tkhai wrote:
>> On 24.12.2018 15:54, Pavel Butsykin wrote:
>>> Fallocate without mutex lock can race with setattr size request, as a
>>> result,
>>> may be various problems, including incorrectly changed file size. At the
>>> same
>>> time pure FALLOC_FL_KEEP_SIZE for vstorage is just nope, so we can
>>> immediately
>>
>> man 2 fallocate:
>>
>>"Specifying the FALLOC_FL_ZERO_RANGE flag (available since Linux 3.15)
>> in mode zeros space in the byte range starting at offset and
>> continuing for len bytes. Within the specified range, blocks are
>> preallocated for the regions that span the holes in the file.
>> After a successful call, subsequent reads from this range will return
>> zeros".
>>
>> So, this patch makes pcs_fuse_kdirect() complete a request without error
>> and without content changing, while userspace thinks the range was zeroed.
>> It's wrong.
>
> I don't quite understand what case you're considering, which breaks
> after my patch. fallocate with mode =
> FALLOC_FL_KEEP_SIZE|FALLOC_FL_ZERO_RANGE
> and offset > file_size, should have no effect on the file.
Yes, everything looks OK for me now.
Reviewed-by: Kirill Tkhai
>>> complete fallocate with mode == FALLOC_FL_KEEP_SIZE. Also move
>>> mutex_is_locked,
>>> since all other mode combinations either have to be under mutex or not
>>> valid.
>>>
>>> #VSTOR-19317
>>>
>>> Signed-off-by: Pavel Butsykin
>>> ---
>>> fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 6 --
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> index de54fedeb5e4..89866370a341 100644
>>> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> @@ -924,9 +924,11 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster
>>> *pfc, struct fuse_req *req,
>>> if (inarg->offset >= di->fileinfo.attr.size)
>>> inarg->mode &= ~FALLOC_FL_ZERO_RANGE;
>>>
>>> + if (inarg->mode == FALLOC_FL_KEEP_SIZE)
>>> + break; /* NOPE */
>>
>> This is untidy to add new branch, which makes further "if (inarg->mode &
>> FALLOC_FL_KEEP_SIZE)"
>> check a dead code.
>>
>>> +
>>> + WARN_ON_ONCE(!mutex_is_locked(>inode.i_mutex));
>>> if (inarg->mode & (FALLOC_FL_ZERO_RANGE|FALLOC_FL_PUNCH_HOLE)) {
>>> - WARN_ON_ONCE(!mutex_is_locked(>inode.i_mutex));
>>> -
>>> if ((inarg->offset & (PAGE_SIZE - 1)) || (inarg->length
>>> & (PAGE_SIZE - 1))) {
>>> r->req.out.h.error = -EINVAL;
>>> goto error;
>>>
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] fs/fuse kio: satisfy pure FALLOC_FL_KEEP_SIZE immediately
On 25.12.2018 12:28, Kirill Tkhai wrote:
> On 24.12.2018 15:54, Pavel Butsykin wrote:
>> Fallocate without mutex lock can race with setattr size request, as a result,
>> may be various problems, including incorrectly changed file size. At the same
>> time pure FALLOC_FL_KEEP_SIZE for vstorage is just nope, so we can
>> immediately
>
> man 2 fallocate:
>
>"Specifying the FALLOC_FL_ZERO_RANGE flag (available since Linux 3.15) in
> mode zeros space in the byte range starting at offset and
> continuing for len bytes. Within the specified range, blocks are
> preallocated for the regions that span the holes in the file.
> After a successful call, subsequent reads from this range will return
> zeros".
>
> So, this patch makes pcs_fuse_kdirect() complete a request without error
> and without content changing, while userspace thinks the range was zeroed.
> It's wrong.
I don't quite understand what case you're considering, which breaks
after my patch. fallocate with mode =
FALLOC_FL_KEEP_SIZE|FALLOC_FL_ZERO_RANGE
and offset > file_size, should have no effect on the file.
>> complete fallocate with mode == FALLOC_FL_KEEP_SIZE. Also move
>> mutex_is_locked,
>> since all other mode combinations either have to be under mutex or not valid.
>>
>> #VSTOR-19317
>>
>> Signed-off-by: Pavel Butsykin
>> ---
>> fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 6 --
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> index de54fedeb5e4..89866370a341 100644
>> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> @@ -924,9 +924,11 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster
>> *pfc, struct fuse_req *req,
>> if (inarg->offset >= di->fileinfo.attr.size)
>> inarg->mode &= ~FALLOC_FL_ZERO_RANGE;
>>
>> +if (inarg->mode == FALLOC_FL_KEEP_SIZE)
>> +break; /* NOPE */
>
> This is untidy to add new branch, which makes further "if (inarg->mode &
> FALLOC_FL_KEEP_SIZE)"
> check a dead code.
>
>> +
>> +WARN_ON_ONCE(!mutex_is_locked(>inode.i_mutex));
>> if (inarg->mode & (FALLOC_FL_ZERO_RANGE|FALLOC_FL_PUNCH_HOLE)) {
>> -WARN_ON_ONCE(!mutex_is_locked(>inode.i_mutex));
>> -
>> if ((inarg->offset & (PAGE_SIZE - 1)) || (inarg->length
>> & (PAGE_SIZE - 1))) {
>> r->req.out.h.error = -EINVAL;
>> goto error;
>>
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] fs/fuse kio: satisfy pure FALLOC_FL_KEEP_SIZE immediately
On 25.12.2018 12:45, Pavel Butsykin wrote:
>
>
> On 25.12.2018 12:28, Kirill Tkhai wrote:
>> On 24.12.2018 15:54, Pavel Butsykin wrote:
>>> Fallocate without mutex lock can race with setattr size request, as a
>>> result,
>>> may be various problems, including incorrectly changed file size. At the
>>> same
>>> time pure FALLOC_FL_KEEP_SIZE for vstorage is just nope, so we can
>>> immediately
>>
>> man 2 fallocate:
>>
>>"Specifying the FALLOC_FL_ZERO_RANGE flag (available since Linux 3.15)
>> in mode zeros space in the byte range starting at offset and
>> continuing for len bytes. Within the specified range, blocks are
>> preallocated for the regions that span the holes in the file.
>> After a successful call, subsequent reads from this range will return
>> zeros".
>
> Yes, FALLOC_FL_ZERO_RANGE | FALLOC_FL_ZERO_RANGE mode combinations
> should always be under mutex, so we can just move my check before:
> if (inarg->offset >= di->fileinfo.attr.size)
> inarg->mode &= ~FALLOC_FL_ZERO_RANGE;
>
>> So, this patch makes pcs_fuse_kdirect() complete a request without error
>> and without content changing, while userspace thinks the range was zeroed.
>> It's wrong.
>>
>>> complete fallocate with mode == FALLOC_FL_KEEP_SIZE. Also move
>>> mutex_is_locked,
>>> since all other mode combinations either have to be under mutex or not
>>> valid.
>>>
>>> #VSTOR-19317
>>>
>>> Signed-off-by: Pavel Butsykin
>>> ---
>>> fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 6 --
>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> index de54fedeb5e4..89866370a341 100644
>>> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>>> @@ -924,9 +924,11 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster
>>> *pfc, struct fuse_req *req,
>>> if (inarg->offset >= di->fileinfo.attr.size)
>>> inarg->mode &= ~FALLOC_FL_ZERO_RANGE;
>>>
>>> + if (inarg->mode == FALLOC_FL_KEEP_SIZE)
>>> + break; /* NOPE */
>>
>> This is untidy to add new branch, which makes further "if (inarg->mode &
>> FALLOC_FL_KEEP_SIZE)"
>> check a dead code.
>
> Why? mode = FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE won't pass my
> check, but will pass the check "if (inarg->mode & FALLOC_FL_KEEP_SIZE)",
> it's not a dead code.
Oh, I misread that. Yeah, there is no problem.
>>> +
>>> + WARN_ON_ONCE(!mutex_is_locked(>inode.i_mutex));
>>> if (inarg->mode & (FALLOC_FL_ZERO_RANGE|FALLOC_FL_PUNCH_HOLE)) {
>>> - WARN_ON_ONCE(!mutex_is_locked(>inode.i_mutex));
>>> -
>>> if ((inarg->offset & (PAGE_SIZE - 1)) || (inarg->length
>>> & (PAGE_SIZE - 1))) {
>>> r->req.out.h.error = -EINVAL;
>>> goto error;
>>>
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] fs/fuse kio: satisfy pure FALLOC_FL_KEEP_SIZE immediately
On 25.12.2018 12:28, Kirill Tkhai wrote:
> On 24.12.2018 15:54, Pavel Butsykin wrote:
>> Fallocate without mutex lock can race with setattr size request, as a result,
>> may be various problems, including incorrectly changed file size. At the same
>> time pure FALLOC_FL_KEEP_SIZE for vstorage is just nope, so we can
>> immediately
>
> man 2 fallocate:
>
>"Specifying the FALLOC_FL_ZERO_RANGE flag (available since Linux 3.15) in
> mode zeros space in the byte range starting at offset and
> continuing for len bytes. Within the specified range, blocks are
> preallocated for the regions that span the holes in the file.
> After a successful call, subsequent reads from this range will return
> zeros".
Yes, FALLOC_FL_ZERO_RANGE | FALLOC_FL_ZERO_RANGE mode combinations
should always be under mutex, so we can just move my check before:
if (inarg->offset >= di->fileinfo.attr.size)
inarg->mode &= ~FALLOC_FL_ZERO_RANGE;
> So, this patch makes pcs_fuse_kdirect() complete a request without error
> and without content changing, while userspace thinks the range was zeroed.
> It's wrong.
>
>> complete fallocate with mode == FALLOC_FL_KEEP_SIZE. Also move
>> mutex_is_locked,
>> since all other mode combinations either have to be under mutex or not valid.
>>
>> #VSTOR-19317
>>
>> Signed-off-by: Pavel Butsykin
>> ---
>> fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 6 --
>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> index de54fedeb5e4..89866370a341 100644
>> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
>> @@ -924,9 +924,11 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster
>> *pfc, struct fuse_req *req,
>> if (inarg->offset >= di->fileinfo.attr.size)
>> inarg->mode &= ~FALLOC_FL_ZERO_RANGE;
>>
>> +if (inarg->mode == FALLOC_FL_KEEP_SIZE)
>> +break; /* NOPE */
>
> This is untidy to add new branch, which makes further "if (inarg->mode &
> FALLOC_FL_KEEP_SIZE)"
> check a dead code.
Why? mode = FALLOC_FL_PUNCH_HOLE|FALLOC_FL_KEEP_SIZE won't pass my
check, but will pass the check "if (inarg->mode & FALLOC_FL_KEEP_SIZE)",
it's not a dead code.
>> +
>> +WARN_ON_ONCE(!mutex_is_locked(>inode.i_mutex));
>> if (inarg->mode & (FALLOC_FL_ZERO_RANGE|FALLOC_FL_PUNCH_HOLE)) {
>> -WARN_ON_ONCE(!mutex_is_locked(>inode.i_mutex));
>> -
>> if ((inarg->offset & (PAGE_SIZE - 1)) || (inarg->length
>> & (PAGE_SIZE - 1))) {
>> r->req.out.h.error = -EINVAL;
>> goto error;
>>
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
Re: [Devel] [PATCH] fs/fuse kio: satisfy pure FALLOC_FL_KEEP_SIZE immediately
On 24.12.2018 15:54, Pavel Butsykin wrote:
> Fallocate without mutex lock can race with setattr size request, as a result,
> may be various problems, including incorrectly changed file size. At the same
> time pure FALLOC_FL_KEEP_SIZE for vstorage is just nope, so we can immediately
man 2 fallocate:
"Specifying the FALLOC_FL_ZERO_RANGE flag (available since Linux 3.15) in
mode zeros space in the byte range starting at offset and
continuing for len bytes. Within the specified range, blocks are
preallocated for the regions that span the holes in the file.
After a successful call, subsequent reads from this range will return zeros".
So, this patch makes pcs_fuse_kdirect() complete a request without error
and without content changing, while userspace thinks the range was zeroed.
It's wrong.
> complete fallocate with mode == FALLOC_FL_KEEP_SIZE. Also move
> mutex_is_locked,
> since all other mode combinations either have to be under mutex or not valid.
>
> #VSTOR-19317
>
> Signed-off-by: Pavel Butsykin
> ---
> fs/fuse/kio/pcs/pcs_fuse_kdirect.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> index de54fedeb5e4..89866370a341 100644
> --- a/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> +++ b/fs/fuse/kio/pcs/pcs_fuse_kdirect.c
> @@ -924,9 +924,11 @@ static void pcs_fuse_submit(struct pcs_fuse_cluster
> *pfc, struct fuse_req *req,
> if (inarg->offset >= di->fileinfo.attr.size)
> inarg->mode &= ~FALLOC_FL_ZERO_RANGE;
>
> + if (inarg->mode == FALLOC_FL_KEEP_SIZE)
> + break; /* NOPE */
This is untidy to add new branch, which makes further "if (inarg->mode &
FALLOC_FL_KEEP_SIZE)"
check a dead code.
> +
> + WARN_ON_ONCE(!mutex_is_locked(>inode.i_mutex));
> if (inarg->mode & (FALLOC_FL_ZERO_RANGE|FALLOC_FL_PUNCH_HOLE)) {
> - WARN_ON_ONCE(!mutex_is_locked(>inode.i_mutex));
> -
> if ((inarg->offset & (PAGE_SIZE - 1)) || (inarg->length
> & (PAGE_SIZE - 1))) {
> r->req.out.h.error = -EINVAL;
> goto error;
>
___
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
