diff mbox series

[v13,39/45] sg: add mmap_sz tracking

Message ID 20210113224526.861000-40-dgilbert@interlog.com
State New
Headers show
Series None | expand

Commit Message

Douglas Gilbert Jan. 13, 2021, 10:45 p.m. UTC 
Track mmap_sz from prior mmap(2) call, per sg file descriptor. Also
reset this value whenever munmap(2) is called. Fail SG_FLAG_MMAP_IO
uses if mmap(2) hasn't been called or the memory associated with it
is not large enough for the current request.

Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
---
 drivers/scsi/sg.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Hannes Reinecke Jan. 14, 2021, 7:18 a.m. UTC | #1 
On 1/13/21 11:45 PM, Douglas Gilbert wrote:
> Track mmap_sz from prior mmap(2) call, per sg file descriptor. Also
> reset this value whenever munmap(2) is called. Fail SG_FLAG_MMAP_IO
> uses if mmap(2) hasn't been called or the memory associated with it
> is not large enough for the current request.
> 
> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
> ---
>   drivers/scsi/sg.c | 11 +++++++++--
>   1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
> index c5533bbaf0d5..d0f25f8f572b 100644
> --- a/drivers/scsi/sg.c
> +++ b/drivers/scsi/sg.c
> @@ -232,6 +232,7 @@ struct sg_fd {		/* holds the state of a file descriptor */
>   	atomic_t waiting;	/* number of requests awaiting receive */
>   	atomic_t req_cnt;	/* number of requests */
>   	int sgat_elem_sz;	/* initialized to scatter_elem_sz */
> +	int mmap_sz;		/* byte size of previous mmap() call */
>   	unsigned long ffd_bm[1];	/* see SG_FFD_* defines above */
>   	pid_t tid;		/* thread id when opened */
>   	u8 next_cmd_len;	/* 0: automatic, >0: use on next write() */
> @@ -725,10 +726,14 @@ sg_write(struct file *filp, const char __user *p, size_t count, loff_t *ppos)
>   static inline int
>   sg_chk_mmap(struct sg_fd *sfp, int rq_flags, int len)
>   {
> +	if (!test_bit(SG_FFD_MMAP_CALLED, sfp->ffd_bm))
> +		return -EBADFD;
>   	if (atomic_read(&sfp->submitted) > 0)
>   		return -EBUSY;  /* already active requests on fd */
>   	if (len > sfp->rsv_srp->sgat_h.buflen)
>   		return -ENOMEM; /* MMAP_IO size must fit in reserve */
> +	if (unlikely(len > sfp->mmap_sz))
> +		return -ENOMEM; /* MMAP_IO size can't exceed mmap() size */
>   	if (rq_flags & SG_FLAG_DIRECT_IO)
>   		return -EINVAL; /* not both MMAP_IO and DIRECT_IO */
>   	return 0;
> @@ -2262,6 +2267,8 @@ sg_vma_close(struct vm_area_struct *vma)
>   		pr_warn("%s: sfp null\n", __func__);
>   		return;
>   	}
> +	sfp->mmap_sz = 0;
> +	clear_bit(SG_FFD_MMAP_CALLED, sfp->ffd_bm);
>   	kref_put(&sfp->f_ref, sg_remove_sfp); /* get in: sg_vma_open() */
>   }
>   
> @@ -2383,6 +2390,7 @@ sg_mmap(struct file *filp, struct vm_area_struct *vma)
>   			goto fini;
>   		}
>   	}
> +	sfp->mmap_sz = req_sz;
>   	vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
>   	vma->vm_private_data = sfp;
>   	vma->vm_ops = &sg_mmap_vm_ops;
> @@ -4048,8 +4056,7 @@ sg_proc_debug_fd(struct sg_fd *fp, char *obp, int len, unsigned long idx)
>   		       (int)test_bit(SG_FFD_FORCE_PACKID, fp->ffd_bm),
>   		       (int)test_bit(SG_FFD_KEEP_ORPHAN, fp->ffd_bm),
>   		       fp->ffd_bm[0]);
> -	n += scnprintf(obp + n, len - n, "   mmap_called=%d\n",
> -		       test_bit(SG_FFD_MMAP_CALLED, fp->ffd_bm));
> +	n += scnprintf(obp + n, len - n, "   mmap_sz=%d\n", fp->mmap_sz);
>   	n += scnprintf(obp + n, len - n,
>   		       "   submitted=%d waiting=%d   open thr_id=%d\n",
>   		       atomic_read(&fp->submitted),
> 
What is the point of having the SG_FFD_MMAP_CALLED bit now?
Doesn't the new 'mmap_sz' value carry the same information?

Cheers,

Hannes
Douglas Gilbert Jan. 14, 2021, 9:40 p.m. UTC | #2 
On 2021-01-14 2:18 a.m., Hannes Reinecke wrote:
> On 1/13/21 11:45 PM, Douglas Gilbert wrote:
>> Track mmap_sz from prior mmap(2) call, per sg file descriptor. Also
>> reset this value whenever munmap(2) is called. Fail SG_FLAG_MMAP_IO
>> uses if mmap(2) hasn't been called or the memory associated with it
>> is not large enough for the current request.
>>
>> Signed-off-by: Douglas Gilbert <dgilbert@interlog.com>
>> ---
>>   drivers/scsi/sg.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
>> index c5533bbaf0d5..d0f25f8f572b 100644
>> --- a/drivers/scsi/sg.c
>> +++ b/drivers/scsi/sg.c
>> @@ -232,6 +232,7 @@ struct sg_fd {        /* holds the state of a file 
>> descriptor */
>>       atomic_t waiting;    /* number of requests awaiting receive */
>>       atomic_t req_cnt;    /* number of requests */
>>       int sgat_elem_sz;    /* initialized to scatter_elem_sz */
>> +    int mmap_sz;        /* byte size of previous mmap() call */
>>       unsigned long ffd_bm[1];    /* see SG_FFD_* defines above */
>>       pid_t tid;        /* thread id when opened */
>>       u8 next_cmd_len;    /* 0: automatic, >0: use on next write() */
>> @@ -725,10 +726,14 @@ sg_write(struct file *filp, const char __user *p, size_t 
>> count, loff_t *ppos)
>>   static inline int
>>   sg_chk_mmap(struct sg_fd *sfp, int rq_flags, int len)
>>   {
>> +    if (!test_bit(SG_FFD_MMAP_CALLED, sfp->ffd_bm))
>> +        return -EBADFD;
>>       if (atomic_read(&sfp->submitted) > 0)
>>           return -EBUSY;  /* already active requests on fd */
>>       if (len > sfp->rsv_srp->sgat_h.buflen)
>>           return -ENOMEM; /* MMAP_IO size must fit in reserve */
>> +    if (unlikely(len > sfp->mmap_sz))
>> +        return -ENOMEM; /* MMAP_IO size can't exceed mmap() size */
>>       if (rq_flags & SG_FLAG_DIRECT_IO)
>>           return -EINVAL; /* not both MMAP_IO and DIRECT_IO */
>>       return 0;
>> @@ -2262,6 +2267,8 @@ sg_vma_close(struct vm_area_struct *vma)
>>           pr_warn("%s: sfp null\n", __func__);
>>           return;
>>       }
>> +    sfp->mmap_sz = 0;
>> +    clear_bit(SG_FFD_MMAP_CALLED, sfp->ffd_bm);
>>       kref_put(&sfp->f_ref, sg_remove_sfp); /* get in: sg_vma_open() */
>>   }
>> @@ -2383,6 +2390,7 @@ sg_mmap(struct file *filp, struct vm_area_struct *vma)
>>               goto fini;
>>           }
>>       }
>> +    sfp->mmap_sz = req_sz;
>>       vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
>>       vma->vm_private_data = sfp;
>>       vma->vm_ops = &sg_mmap_vm_ops;
>> @@ -4048,8 +4056,7 @@ sg_proc_debug_fd(struct sg_fd *fp, char *obp, int len, 
>> unsigned long idx)
>>                  (int)test_bit(SG_FFD_FORCE_PACKID, fp->ffd_bm),
>>                  (int)test_bit(SG_FFD_KEEP_ORPHAN, fp->ffd_bm),
>>                  fp->ffd_bm[0]);
>> -    n += scnprintf(obp + n, len - n, "   mmap_called=%d\n",
>> -               test_bit(SG_FFD_MMAP_CALLED, fp->ffd_bm));
>> +    n += scnprintf(obp + n, len - n, "   mmap_sz=%d\n", fp->mmap_sz);
>>       n += scnprintf(obp + n, len - n,
>>                  "   submitted=%d waiting=%d   open thr_id=%d\n",
>>                  atomic_read(&fp->submitted),
>>
> What is the point of having the SG_FFD_MMAP_CALLED bit now?
> Doesn't the new 'mmap_sz' value carry the same information?

Touché! Will fix.

Doug Gilbert
diff mbox series

Patch

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index c5533bbaf0d5..d0f25f8f572b 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -232,6 +232,7 @@  struct sg_fd {		/* holds the state of a file descriptor */
 	atomic_t waiting;	/* number of requests awaiting receive */
 	atomic_t req_cnt;	/* number of requests */
 	int sgat_elem_sz;	/* initialized to scatter_elem_sz */
+	int mmap_sz;		/* byte size of previous mmap() call */
 	unsigned long ffd_bm[1];	/* see SG_FFD_* defines above */
 	pid_t tid;		/* thread id when opened */
 	u8 next_cmd_len;	/* 0: automatic, >0: use on next write() */
@@ -725,10 +726,14 @@  sg_write(struct file *filp, const char __user *p, size_t count, loff_t *ppos)
 static inline int
 sg_chk_mmap(struct sg_fd *sfp, int rq_flags, int len)
 {
+	if (!test_bit(SG_FFD_MMAP_CALLED, sfp->ffd_bm))
+		return -EBADFD;
 	if (atomic_read(&sfp->submitted) > 0)
 		return -EBUSY;  /* already active requests on fd */
 	if (len > sfp->rsv_srp->sgat_h.buflen)
 		return -ENOMEM; /* MMAP_IO size must fit in reserve */
+	if (unlikely(len > sfp->mmap_sz))
+		return -ENOMEM; /* MMAP_IO size can't exceed mmap() size */
 	if (rq_flags & SG_FLAG_DIRECT_IO)
 		return -EINVAL; /* not both MMAP_IO and DIRECT_IO */
 	return 0;
@@ -2262,6 +2267,8 @@  sg_vma_close(struct vm_area_struct *vma)
 		pr_warn("%s: sfp null\n", __func__);
 		return;
 	}
+	sfp->mmap_sz = 0;
+	clear_bit(SG_FFD_MMAP_CALLED, sfp->ffd_bm);
 	kref_put(&sfp->f_ref, sg_remove_sfp); /* get in: sg_vma_open() */
 }
 
@@ -2383,6 +2390,7 @@  sg_mmap(struct file *filp, struct vm_area_struct *vma)
 			goto fini;
 		}
 	}
+	sfp->mmap_sz = req_sz;
 	vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
 	vma->vm_private_data = sfp;
 	vma->vm_ops = &sg_mmap_vm_ops;
@@ -4048,8 +4056,7 @@  sg_proc_debug_fd(struct sg_fd *fp, char *obp, int len, unsigned long idx)
 		       (int)test_bit(SG_FFD_FORCE_PACKID, fp->ffd_bm),
 		       (int)test_bit(SG_FFD_KEEP_ORPHAN, fp->ffd_bm),
 		       fp->ffd_bm[0]);
-	n += scnprintf(obp + n, len - n, "   mmap_called=%d\n",
-		       test_bit(SG_FFD_MMAP_CALLED, fp->ffd_bm));
+	n += scnprintf(obp + n, len - n, "   mmap_sz=%d\n", fp->mmap_sz);
 	n += scnprintf(obp + n, len - n,
 		       "   submitted=%d waiting=%d   open thr_id=%d\n",
 		       atomic_read(&fp->submitted),