On 10/14/24 1:08 PM, Jann Horn wrote:
On Wed, Sep 4, 2024 at 1:22 AM Anthony Yznaga <anthony.yznaga@xxxxxxxxxx> wrote:
Reserve a range of ioctls for msharefs and add the first two ioctls
to get and set the start address and size of an mshare region.
[...]
+static long
+msharefs_set_size(struct mm_struct *mm, struct mshare_data *m_data,
+ struct mshare_info *minfo)
+{
+ unsigned long end = minfo->start + minfo->size;
+
+ /*
+ * Validate alignment for start address, and size
+ */
+ if ((minfo->start | end) & (PGDIR_SIZE - 1)) {
+ spin_unlock(&m_data->m_lock);
+ return -EINVAL;
+ }
+
+ mm->mmap_base = minfo->start;
+ mm->task_size = minfo->size;
+ if (!mm->task_size)
+ mm->task_size--;
+
+ m_data->minfo.start = mm->mmap_base;
+ m_data->minfo.size = mm->task_size;
+ spin_unlock(&m_data->m_lock);
+
+ return 0;
+}
+
+static long
+msharefs_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+ struct mshare_data *m_data = filp->private_data;
+ struct mm_struct *mm = m_data->mm;
+ struct mshare_info minfo;
+
+ switch (cmd) {
+ case MSHAREFS_GET_SIZE:
+ spin_lock(&m_data->m_lock);
+ minfo = m_data->minfo;
+ spin_unlock(&m_data->m_lock);
+
+ if (copy_to_user((void __user *)arg, &minfo, sizeof(minfo)))
+ return -EFAULT;
+
+ return 0;
+
+ case MSHAREFS_SET_SIZE:
+ if (copy_from_user(&minfo, (struct mshare_info __user *)arg,
+ sizeof(minfo)))
+ return -EFAULT;
+
+ /*
+ * If this mshare region has been set up once already, bail out
+ */
+ spin_lock(&m_data->m_lock);
+ if (m_data->minfo.start != 0) {
Is there actually anything that prevents msharefs_set_size() from
setting up m_data with ->minfo.start==0, so that a second
MSHAREFS_SET_SIZE invocation will succeed? It would probably be more
reliable to have a separate flag for "has this thing been set up yet".
Thanks for pointing this out. Yes, this is problematic. A start address
of 0 generally won't work because mmap() will fail unless there are
sufficient privileges (cap_map_addr will return -EPERM). I already have
changes to use the size to indicate initialization, but it may make
sense to have flags.
Anthony
+ spin_unlock(&m_data->m_lock);
+ return -EINVAL;
+ }
+
+ return msharefs_set_size(mm, m_data, &minfo);
+
+ default:
+ return -ENOTTY;
+ }
+}
