From: "Alexey Gladkov (Intel)" <legion@kernel.org>
In case the instruction is close to the page boundary, reading
MAX_INSN_SIZE may cross the page boundary. The second page might be
from a different VMA and reading can have side effects.
The problem is that the actual size of the instruction is not known.
The solution might be to try read the data to the end of the page and
try parse it in the hope that the instruction is smaller than the
maximum buffer size.
Co-developed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Signed-off-by: Alexey Gladkov (Intel) <legion@kernel.org>
---
arch/x86/include/asm/insn-eval.h | 15 +++++++++
arch/x86/lib/insn-eval.c | 55 ++++++++++++++++++++++++++++++++
2 files changed, 70 insertions(+)
diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
index 54368a43abf6..160e483bde99 100644
--- a/arch/x86/include/asm/insn-eval.h
+++ b/arch/x86/include/asm/insn-eval.h
@@ -32,6 +32,21 @@ int insn_fetch_from_user_inatomic(struct pt_regs *regs,
bool insn_decode_from_regs(struct insn *insn, struct pt_regs *regs,
unsigned char buf[MAX_INSN_SIZE], int buf_size);
+int insn_fetch_decode_from_user_common(struct insn *insn, struct pt_regs *regs,
+ bool inatomic);
+
+static inline int insn_fetch_decode_from_user(struct insn *insn,
+ struct pt_regs *regs)
+{
+ return insn_fetch_decode_from_user_common(insn, regs, false);
+}
+
+static inline int insn_fetch_decode_from_user_inatomic(struct insn *insn,
+ struct pt_regs *regs)
+{
+ return insn_fetch_decode_from_user_common(insn, regs, true);
+}
+
enum insn_mmio_type {
INSN_MMIO_DECODE_FAILED,
INSN_MMIO_WRITE,
diff --git a/arch/x86/lib/insn-eval.c b/arch/x86/lib/insn-eval.c
index 98631c0e7a11..67bfb645df67 100644
--- a/arch/x86/lib/insn-eval.c
+++ b/arch/x86/lib/insn-eval.c
@@ -1668,3 +1668,58 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes)
return type;
}
+
+/**
+ * insn_fetch_decode_from_user_common() - Copy and decode instruction bytes
+ * from user-space memory
+ * @buf: Array to store the fetched instruction
+ * @regs: Structure with register values as seen when entering kernel mode
+ * @inatomic boolean flag whether function is used in atomic context
+ *
+ * Gets the linear address of the instruction and copies the instruction bytes
+ * and decodes the instruction.
+ *
+ * Returns:
+ *
+ * - 0 on success.
+ * - -EFAULT if the copy from userspace fails.
+ * - -EINVAL if the linear address of the instruction could not be calculated.
+ */
+int insn_fetch_decode_from_user_common(struct insn *insn, struct pt_regs *regs,
+ bool inatomic)
+{
+ char buffer[MAX_INSN_SIZE];
+ int nr_copied, size;
+ unsigned long ip;
+
+ if (insn_get_effective_ip(regs, &ip))
+ return -EINVAL;
+
+ /*
+ * On the first attempt, read up to MAX_INSN_SIZE, but do not cross a
+ * page boundary. The second page might be from a different VMA and
+ * reading can have side effects (i.e. reading from MMIO).
+ */
+ size = min(MAX_INSN_SIZE, PAGE_SIZE - offset_in_page(ip));
+retry:
+ nr_copied = size;
+
+ if (inatomic)
+ nr_copied -= __copy_from_user_inatomic(buffer, (void __user *)ip, size);
+ else
+ nr_copied -= copy_from_user(buffer, (void __user *)ip, size);
+
+ if (nr_copied <= 0)
+ return -EFAULT;
+
+ if (!insn_decode_from_regs(insn, regs, buffer, nr_copied)) {
+ /* If decode failed, try to copy across page boundary */
+ if (size < MAX_INSN_SIZE) {
+ size = MAX_INSN_SIZE;
+ goto retry;
+ }
+ return -EINVAL;
+ }
+
+ return 0;
+}
--
2.45.2On Fri, Aug 16, 2024 at 03:43:54PM +0200, Alexey Gladkov wrote: > From: "Alexey Gladkov (Intel)" <legion@kernel.org> > > In case the instruction is close to the page boundary, reading > MAX_INSN_SIZE may cross the page boundary. The second page might be > from a different VMA and reading can have side effects. > > The problem is that the actual size of the instruction is not known. > > The solution might be to try read the data to the end of the page and > try parse it in the hope that the instruction is smaller than the > maximum buffer size. > > Co-developed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > Signed-off-by: Alexey Gladkov (Intel) <legion@kernel.org> I think this and 3 next patches do not belong to this patchset. They address separate issue that is orthogonal to the patchset goal. -- Kiryl Shutsemau / Kirill A. Shutemov
On Mon, Aug 19, 2024 at 01:48:11PM +0300, Kirill A. Shutemov wrote: > On Fri, Aug 16, 2024 at 03:43:54PM +0200, Alexey Gladkov wrote: > > From: "Alexey Gladkov (Intel)" <legion@kernel.org> > > > > In case the instruction is close to the page boundary, reading > > MAX_INSN_SIZE may cross the page boundary. The second page might be > > from a different VMA and reading can have side effects. > > > > The problem is that the actual size of the instruction is not known. > > > > The solution might be to try read the data to the end of the page and > > try parse it in the hope that the instruction is smaller than the > > maximum buffer size. > > > > Co-developed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > Signed-off-by: Alexey Gladkov (Intel) <legion@kernel.org> > > I think this and 3 next patches do not belong to this patchset. They > address separate issue that is orthogonal to the patchset goal. Should I drop them from this patchset and send them after this patchset as a separate change ? -- Rgrds, legion
On Mon, Aug 19, 2024 at 01:56:05PM +0200, Alexey Gladkov wrote: > On Mon, Aug 19, 2024 at 01:48:11PM +0300, Kirill A. Shutemov wrote: > > On Fri, Aug 16, 2024 at 03:43:54PM +0200, Alexey Gladkov wrote: > > > From: "Alexey Gladkov (Intel)" <legion@kernel.org> > > > > > > In case the instruction is close to the page boundary, reading > > > MAX_INSN_SIZE may cross the page boundary. The second page might be > > > from a different VMA and reading can have side effects. > > > > > > The problem is that the actual size of the instruction is not known. > > > > > > The solution might be to try read the data to the end of the page and > > > try parse it in the hope that the instruction is smaller than the > > > maximum buffer size. > > > > > > Co-developed-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com> > > > Signed-off-by: Alexey Gladkov (Intel) <legion@kernel.org> > > > > I think this and 3 next patches do not belong to this patchset. They > > address separate issue that is orthogonal to the patchset goal. > > Should I drop them from this patchset and send them after this patchset as > a separate change ? Yeah. I think so. -- Kiryl Shutsemau / Kirill A. Shutemov
Hi Alexey,
kernel test robot noticed the following build warnings:
[auto build test WARNING on tip/x86/core]
[also build test WARNING on tip/master linus/master v6.11-rc3 next-20240816]
[cannot apply to tip/x86/tdx tip/auto-latest]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Alexey-Gladkov/x86-tdx-Split-MMIO-read-and-write-operations/20240816-222615
base: tip/x86/core
patch link: https://lore.kernel.org/r/9704da6a35d62932d464d33b39953fc5b2fd74ea.1723807851.git.legion%40kernel.org
patch subject: [PATCH v3 04/10] x86/insn: Read and decode insn without crossing the page boundary
config: i386-buildonly-randconfig-001-20240817 (https://download.01.org/0day-ci/archive/20240817/202408171001.feB1A8FN-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240817/202408171001.feB1A8FN-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408171001.feB1A8FN-lkp@intel.com/
All warnings (new ones prefixed by >>):
>> arch/x86/lib/insn-eval.c:1690: warning: Function parameter or struct member 'insn' not described in 'insn_fetch_decode_from_user_common'
>> arch/x86/lib/insn-eval.c:1690: warning: Function parameter or struct member 'inatomic' not described in 'insn_fetch_decode_from_user_common'
>> arch/x86/lib/insn-eval.c:1690: warning: Excess function parameter 'buf' description in 'insn_fetch_decode_from_user_common'
vim +1690 arch/x86/lib/insn-eval.c
1671
1672 /**
1673 * insn_fetch_decode_from_user_common() - Copy and decode instruction bytes
1674 * from user-space memory
1675 * @buf: Array to store the fetched instruction
1676 * @regs: Structure with register values as seen when entering kernel mode
1677 * @inatomic boolean flag whether function is used in atomic context
1678 *
1679 * Gets the linear address of the instruction and copies the instruction bytes
1680 * and decodes the instruction.
1681 *
1682 * Returns:
1683 *
1684 * - 0 on success.
1685 * - -EFAULT if the copy from userspace fails.
1686 * - -EINVAL if the linear address of the instruction could not be calculated.
1687 */
1688 int insn_fetch_decode_from_user_common(struct insn *insn, struct pt_regs *regs,
1689 bool inatomic)
> 1690 {
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2026 Red Hat, Inc.