Bug 410556 - [PATCH] add support for BLKIO{MIN,OPT} and BLKALIGNOFF ioctls
Summary: [PATCH] add support for BLKIO{MIN,OPT} and BLKALIGNOFF ioctls
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: unspecified
Platform: Debian unstable Linux
: NOR normal (vote)
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-08-04 01:27 UTC by Nick Black
Modified: 2019-12-30 10:14 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In:


Attachments
Patch to bring blk ioctls up to speed with 5.2.5 (2.72 KB, patch)
2019-08-04 01:27 UTC, Nick Black
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Black 2019-08-04 01:27:12 UTC
Created attachment 121919 [details]
Patch to bring blk ioctls up to speed with 5.2.5

Valgrind as checked out from git://sourceware.org/git/valgrind.git (and forked to 
git@github.com:dankamongmen/valgrind.git) is missing support for three Linux block ioctl()s. This was discovered while developing Growlight (https://github.com/dankamongmen/growlight/issues/8). Without this patch, Valgrind gives a false warning following my BLKALIGNOFF call. With it, there is no such warning.

STEPS TO REPRODUCE
1. Call one of the mentioned ioctl()s under Valgrind
2. Use the result

OBSERVED RESULT
==15964== Warning: noted but unhandled ioctl 0x127a with no size/direction hints.
==15964==    This could cause spurious value errors to appear.
==15964==    See README_MISSING_SYSCALL_OR_IOCTL for guidance on writing a proper wrapper.
==15964== Thread 4:
==15964== Conditional jump or move depends on uninitialised value(s)
==15964==    at 0x4D99883: ??? (in /lib/x86_64-linux-gnu/libblkid.so.1.1.0)
==15964==    by 0x4D9A465: ??? (in /lib/x86_64-linux-gnu/libblkid.so.1.1.0)
==15964==    by 0x4D99A88: ??? (in /lib/x86_64-linux-gnu/libblkid.so.1.1.0)
==15964==    by 0x4D8817A: blkid_do_fullprobe (in /lib/x86_64-linux-gnu/libblkid.so.1.1.0)
==15964==    by 0x114E1E: probe_blkid_superblock (libblkid.c:127)
==15964==    by 0x110CE0: rescan (growlight.c:1014)
==15964==    by 0x110CE0: rescan (growlight.c:905)
==15964==    by 0x1121C5: create_new_device_inner (growlight.c:1137)
==15964==    by 0x1121C5: create_new_device (growlight.c:1179)
==15964==    by 0x112445: lookup_device (growlight.c:1249)
==15964==    by 0x112445: lookup_device (growlight.c:1201)
==15964==    by 0x113C6C: scan_device (growlight.c:1381)
==15964==    by 0x5215FA2: start_thread (pthread_create.c:486)
==15964==    by 0x53284CE: clone (clone.S:95)

EXPECTED RESULT
Smooth sailing

SOFTWARE/OS VERSIONS
Linux: 5.2.5

ADDITIONAL INFORMATION
Please merge. I've contributed once before, an ioctl for CDROM_GET_CAPABILITY back in 2012 (due to the same project, actually).
Comment 1 Nick Black 2019-08-04 01:27:35 UTC
From 0fe56735ca5ddad378126bd2954d93f5dcfbc876 Mon Sep 17 00:00:00 2001
From: nick black <dankamongmen@gmail.com>
Date: Sat, 3 Aug 2019 20:43:30 -0400
Subject: [PATCH] linux-ioctl: add BLKIO{MIN,OPT} and BLKALIGNOFF

Signed-off-by: nick black <dankamongmen@gmail.com>
---
 coregrind/m_syswrap/syswrap-linux.c | 18 ++++++++++++++++++
 include/vki/vki-linux.h             |  5 ++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/coregrind/m_syswrap/syswrap-linux.c b/coregrind/m_syswrap/syswrap-linux.c
index 9d9a1d4ea..a7bc9732b 100644
--- a/coregrind/m_syswrap/syswrap-linux.c
+++ b/coregrind/m_syswrap/syswrap-linux.c
@@ -7236,6 +7236,15 @@ PRE(sys_ioctl)
    case VKI_BLKPBSZGET:
       PRE_MEM_WRITE( "ioctl(BLKPBSZGET)", ARG3, sizeof(int));
       break;
+   case VKI_BLKIOMIN:
+      PRE_MEM_WRITE( "ioctl(BLKIOMIN)", ARG3, sizeof(vki_uint));
+      break;
+   case VKI_BLKIOOPT:
+      PRE_MEM_WRITE( "ioctl(BLKIOOPT)", ARG3, sizeof(vki_uint));
+      break;
+   case VKI_BLKALIGNOFF:
+      PRE_MEM_WRITE( "ioctl(BLKALIGNOFF)", ARG3, sizeof(int));
+      break;
    case VKI_BLKDISCARDZEROES:
       PRE_MEM_WRITE( "ioctl(BLKDISCARDZEROES)", ARG3, sizeof(vki_uint));
       break;
@@ -10072,6 +10081,15 @@ POST(sys_ioctl)
    case VKI_BLKPBSZGET:
       POST_MEM_WRITE(ARG3, sizeof(int));
       break;
+   case VKI_BLKIOMIN:
+      POST_MEM_WRITE(ARG3, sizeof(vki_uint));
+      break;
+   case VKI_BLKIOOPT:
+      POST_MEM_WRITE(ARG3, sizeof(vki_uint));
+      break;
+   case VKI_BLKALIGNOFF:
+      POST_MEM_WRITE(ARG3, sizeof(int));
+      break;
    case VKI_BLKDISCARDZEROES:
       POST_MEM_WRITE(ARG3, sizeof(vki_uint));
       break;
diff --git a/include/vki/vki-linux.h b/include/vki/vki-linux.h
index 76bc7837b..a9bfb1ad0 100644
--- a/include/vki/vki-linux.h
+++ b/include/vki/vki-linux.h
@@ -1813,7 +1813,7 @@ struct vki_ppdev_frob_struct {
 #define VKI_PPSETFLAGS	_VKI_IOW(VKI_PP_IOCTL, 0x9b, int)
 
 //----------------------------------------------------------------------
-// From linux-3.16/include/uapi/linux/fs.h
+// From linux-5.2.5/include/uapi/linux/fs.h
 //----------------------------------------------------------------------
 
 #define VKI_BLKROSET   _VKI_IO(0x12,93)	/* set device read-only (0 = read-write) */
@@ -1832,6 +1832,9 @@ struct vki_ppdev_frob_struct {
 #define VKI_BLKBSZSET  _VKI_IOW(0x12,113,vki_size_t)
 #define VKI_BLKGETSIZE64 _VKI_IOR(0x12,114,vki_size_t) /* return device size in bytes (u64 *arg) */
 #define VKI_BLKDISCARD _VKI_IO(0x12,119)
+#define VKI_BLKIOMIN _VKI_IO(0x12,120)
+#define VKI_BLKIOOPT _VKI_IO(0x12,121)
+#define VKI_BLKALIGNOFF _VKI_IO(0x12,122)
 #define VKI_BLKPBSZGET _VKI_IO(0x12,123)
 #define VKI_BLKDISCARDZEROES _VKI_IO(0x12,124)
 #define VKI_BLKZEROOUT _VKI_IO(0x12,127)
-- 
2.23.0.rc1
Comment 2 Nick Black 2019-09-18 00:07:08 UTC
Hey there! Any chance this can get merged? It seems to work pretty well.
Comment 3 Julian Seward 2019-12-28 16:50:10 UTC
Will be merged in the next month or so, providing it doesn't break anything.
Comment 4 Julian Seward 2019-12-30 10:14:51 UTC
Committed as 11b7891a8a39b0dae39c97cfcd039a708f83a69a.
Thanks for the patch.