[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAG48ez0OOpw17d73wB_HC55FVLeKOz0D9+teEHe7YAsY_00=kw@mail.gmail.com>
Date: Mon, 14 Oct 2024 22:08:38 +0200
From: Jann Horn <jannh@...gle.com>
To: Anthony Yznaga <anthony.yznaga@...cle.com>
Cc: akpm@...ux-foundation.org, willy@...radead.org, markhemm@...glemail.com,
viro@...iv.linux.org.uk, david@...hat.com, khalid@...nel.org,
andreyknvl@...il.com, dave.hansen@...el.com, luto@...nel.org,
brauner@...nel.org, arnd@...db.de, ebiederm@...ssion.com,
catalin.marinas@....com, linux-arch@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-mm@...ck.org, mhiramat@...nel.org,
rostedt@...dmis.org, vasily.averin@...ux.dev, xhao@...ux.alibaba.com,
pcc@...gle.com, neilb@...e.de, maz@...nel.org
Subject: Re: [RFC PATCH v3 05/10] mm/mshare: Add ioctl support
On Wed, Sep 4, 2024 at 1:22 AM Anthony Yznaga <anthony.yznaga@...cle.com> 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".
> + spin_unlock(&m_data->m_lock);
> + return -EINVAL;
> + }
> +
> + return msharefs_set_size(mm, m_data, &minfo);
> +
> + default:
> + return -ENOTTY;
> + }
> +}
Powered by blists - more mailing lists