Skip to contentSkip to conversation

ptrdiff_t is not uintptr_t, damnit

PATCH In review Al Viro 4 participants 13 messages Aug 19, 2007 – Aug 21, 2007

Subsystems (4)
SCSI SPI NTFS filesystem Aacraid SCSI RAID driver
lore
Share & tools
lore

Al Viro

author
#0 lore raw

PATCH 1/1 ptrdiff_t is not uintptr_t, damnit

Use of ptrdiff_t in

- if (!access_ok(VERIFY_WRITE, u_tmp->rx_buf, u_tmp->len))
+ if (!access_ok(VERIFY_WRITE, (u8 __user *)
+ (ptrdiff_t) u_tmp->rx_buf,
+ u_tmp->len))

is wrong; for one thing, it's a bad C (it's what uintptr_t is for; in general we are not even promised that ptrdiff_t is large enough to hold a pointer, just enough to hold a difference between two pointers within the same object). For another, it confuses the fsck out of sparse.

Use unsigned long or uintptr_t instead. There are several places misusing ptrdiff_t (one - in a very odd way; WTF did that cast to __user ptrdiff_t
in ntfs expect to happen, anyway?) Fixed...

Signed-off-by Al Viro <[email protected]>

---

drivers/scsi/aacraid/commctrl.c +6 -6
index 72b0393..1e6d7a9 100644
@@ -391,7 +391,7 @@ static int close_getadapter_fib(struct aac_dev * dev, void __user *arg)
 		/*
 		 *	Extract the fibctx from the input parameters
 		 */
-		if (fibctx->unique == (u32)(ptrdiff_t)arg) /* We found a winner */
+		if (fibctx->unique == (u32)(uintptr_t)arg) /* We found a winner */
 			break;
 		entry = entry->next;
 		fibctx = NULL;

@@ -590,7 +590,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 				}
 				addr = (u64)upsg->sg[i].addr[0];
 				addr += ((u64)upsg->sg[i].addr[1]) << 32;
-				sg_user[i] = (void __user *)(ptrdiff_t)addr;
+				sg_user[i] = (void __user *)(uintptr_t)addr;
 				sg_list[i] = p; // save so we can clean up later
 				sg_indx = i;
 

@@ -633,7 +633,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 					rcode = -ENOMEM;
 					goto cleanup;
 				}
-				sg_user[i] = (void __user *)(ptrdiff_t)usg->sg[i].addr;
+				sg_user[i] = (void __user *)(uintptr_t)usg->sg[i].addr;
 				sg_list[i] = p; // save so we can clean up later
 				sg_indx = i;
 

@@ -664,7 +664,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 		if (actual_fibsize64 == fibsize) {
 			struct user_sgmap64* usg = (struct user_sgmap64 *)upsg;
 			for (i = 0; i < upsg->count; i++) {
-				u64 addr;
+				uintptr_t addr;
 				void* p;
 				/* Does this really need to be GFP_DMA? */
 				p = kmalloc(usg->sg[i].count,GFP_KERNEL|__GFP_DMA);

@@ -676,7 +676,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 				}
 				addr = (u64)usg->sg[i].addr[0];
 				addr += ((u64)usg->sg[i].addr[1]) << 32;
-				sg_user[i] = (void __user *)(ptrdiff_t)addr;
+				sg_user[i] = (void __user *)addr;
 				sg_list[i] = p; // save so we can clean up later
 				sg_indx = i;
 

@@ -704,7 +704,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg)
 					rcode = -ENOMEM;
 					goto cleanup;
 				}
-				sg_user[i] = (void __user *)(ptrdiff_t)upsg->sg[i].addr;
+				sg_user[i] = (void __user *)(uintptr_t)upsg->sg[i].addr;
 				sg_list[i] = p; // save so we can clean up later
 				sg_indx = i;
drivers/scsi/aacraid/comminit.c +1 -1
index 3009ad8..8736813 100644
@@ -110,7 +110,7 @@ static int aac_alloc_comm(struct aac_dev *dev, void **commaddr, unsigned long co
 	/*
 	 *	Align the beginning of Headers to commalign
 	 */
-	align = (commalign - ((ptrdiff_t)(base) & (commalign - 1)));
+	align = (commalign - ((uintptr_t)(base) & (commalign - 1)));
 	base = base + align;
 	phys = phys + align;
 	/*
drivers/scsi/aacraid/dpcsup.c +1 -1
index fcd25f7..e6032ff 100644
@@ -254,7 +254,7 @@ unsigned int aac_intr_normal(struct aac_dev * dev, u32 Index)
 			kfree (fib);
 			return 1;
 		}
-		memcpy(hw_fib, (struct hw_fib *)(((ptrdiff_t)(dev->regs.sa)) +
+		memcpy(hw_fib, (struct hw_fib *)(((uintptr_t)(dev->regs.sa)) +
 		  (index & ~0x00000002L)), sizeof(struct hw_fib));
 		INIT_LIST_HEAD(&fib->fiblink);
 		fib->type = FSAFS_NTC_FIB_CONTEXT;
drivers/spi/spidev.c +3 -3
index c55459c..b3518ca 100644
@@ -184,14 +184,14 @@ static int spidev_message(struct spidev_data *spidev,
 		if (u_tmp->rx_buf) {
 			k_tmp->rx_buf = buf;
 			if (!access_ok(VERIFY_WRITE, (u8 __user *)
-						(ptrdiff_t) u_tmp->rx_buf,
+						(uintptr_t) u_tmp->rx_buf,
 						u_tmp->len))
 				goto done;
 		}
 		if (u_tmp->tx_buf) {
 			k_tmp->tx_buf = buf;
 			if (copy_from_user(buf, (const u8 __user *)
-						(ptrdiff_t) u_tmp->tx_buf,
+						(uintptr_t) u_tmp->tx_buf,
 					u_tmp->len))
 				goto done;
 		}

@@ -224,7 +224,7 @@ static int spidev_message(struct spidev_data *spidev,
 	for (n = n_xfers, u_tmp = u_xfers; n; n--, u_tmp++) {
 		if (u_tmp->rx_buf) {
 			if (__copy_to_user((u8 __user *)
-					(ptrdiff_t) u_tmp->rx_buf, buf,
+					(uintptr_t) u_tmp->rx_buf, buf,
 					u_tmp->len)) {
 				status = -EFAULT;
 				goto done;
fs/ntfs/file.c +1 -1
index ffcc504..a9103ce 100644
@@ -362,7 +362,7 @@ static inline void ntfs_fault_in_pages_readable(const char __user *uaddr,
 	volatile char c;
 
 	/* Set @end to the first byte outside the last page we care about. */
-	end = (const char __user*)PAGE_ALIGN((ptrdiff_t __user)uaddr + bytes);
+	end = (const char __user*)PAGE_ALIGN((uintptr_t)uaddr + bytes);
 
 	while (!__get_user(c, uaddr) && (uaddr += PAGE_SIZE, uaddr < end))
 		;
include/linux/types.h +2 -0
index 0351bf2..efdec19 100644
@@ -40,6 +40,8 @@ typedef __kernel_gid32_t	gid_t;
 typedef __kernel_uid16_t        uid16_t;
 typedef __kernel_gid16_t        gid16_t;
 
+typedef unsigned long		uintptr_t;
+
 #ifdef CONFIG_UID16
 /* This is defined by include/asm-{arch}/posix_types.h */
 typedef __kernel_old_uid_t	old_uid_t;
#1 lore raw
Quoted 2 lines · Al Viro

is wrong; for one thing, it's a bad C (it's what uintptr_t is for; in general we are not even promised that ptrdiff_t is large enough to hold a pointer,

ISTR we don't *have* a uintptr_t on all architectures, or that would be the appropriate thing to use in these 32/64 bit ABI scenarios.

Quoted 1 line

Use unsigned long or uintptr_t instead.

I suspect you mean "unsigned long long"...

- Dave

#2 lore raw

Hi,

Quoted 11 lines · David Brownell
Quoted 2 lines · Al Viro

is wrong; for one thing, it's a bad C (it's what uintptr_t is for; in general we are not even promised that ptrdiff_t is large enough to hold a pointer,

ISTR we don't *have* a uintptr_t on all architectures, or that would be the appropriate thing to use in these 32/64 bit ABI scenarios.

Quoted 1 line

Use unsigned long or uintptr_t instead.

I suspect you mean "unsigned long long"...

No he doesn't. "unsigned long" is guaranteed to be large enough to hold a pointer (at least on Linux anyway).

On a 32-bit arch "unsigned long" is 32-bit and pointers are 32-bit.

On a 64-bit archi "unsigned long" is 64-bit and pointers are 64-bit.

Best regards,

Anton

Signature · 3 lines
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/
#3 lore raw
Quoted 11 lines · Anton Altaparmakov
Quoted 8 lines

ISTR we don't *have* a uintptr_t on all architectures, or that would be the appropriate thing to use in these 32/64 bit ABI scenarios.

Quoted 1 line

Use unsigned long or uintptr_t instead.

I suspect you mean "unsigned long long"...

No he doesn't.  "unsigned long" is guaranteed to be large enough to hold a pointer (at least on Linux anyway).

And yet when I used that, I got compiler warnings on some systems.

ISTR that was the first solution I tried, but GCC really wanted to issue warnings. Either about casting 64-bit to pointer, or about casting it to "unsigned long", either way lost precision.

Quoted 3 lines

On a 32-bit arch "unsigned long" is 32-bit and pointers are 32-bit.

On a 64-bit archi "unsigned long" is 64-bit and pointers are 64-bit.

So with 32 bit userspace "unsigned long long" is the type to use when talking to a 64-bit kernel; and with pure 64-bit code, it's enough to write "unsigned long".

I'm fairly sure that's the root cause of the pain I recall here; but I'd have to run experiments again to verify that.

#4 lore raw
Quoted 6 lines · David Brownell
Quoted 5 lines · Anton Altaparmakov
Quoted 5 lines

ISTR we don't *have* a uintptr_t on all architectures, or that would be the appropriate thing to use in these 32/64 bit ABI scenarios.

Quoted 6 lines
Quoted 6 lines
Quoted 3 lines
Quoted 1 line

Use unsigned long or uintptr_t instead.

I suspect you mean "unsigned long long"...

No he doesn't.  "unsigned long" is guaranteed to be large enough to  hold a pointer (at least on Linux anyway).

Yup, sizeof(long) >= sizeof(void *) should always be true in Linux C. I bet a lot of code out there depends on this.

BTW, just curious to know, but which (if any) are the platforms that have sizeof(long) > sizeof(void *)? [i.e. greater-than but not equal?]

The reason I ask is that gcc will also complain (understandably so) with "warning: cast from pointer to integer of different size" i.e. even if it's a conversion from smaller size to greater size, and not really a case of truncation. Therefore, I wonder if the stricter assertion:
sizeof(long) == sizeof(void *) holds true for Linux C, actually.

Quoted 5 lines

And yet when I used that, I got compiler warnings on some systems.

ISTR that was the first solution I tried, but GCC really wanted to issue warnings. Either about casting 64-bit to pointer, or about casting it to "unsigned long", either way lost precision.

Hmm, if you could fish out those testcases ...

Quoted 10 lines
Quoted 3 lines

On a 32-bit arch "unsigned long" is 32-bit and pointers are 32-bit.

On a 64-bit archi "unsigned long" is 64-bit and pointers are 64-bit.

So with 32 bit userspace "unsigned long long" is the type to use when talking to a 64-bit kernel; and with pure 64-bit code, it's enough to write "unsigned long".

I'm fairly sure that's the root cause of the pain I recall here; but I'd have to run experiments again to verify that.

I suspect the root cause of the pain was that you used "int" or "long" to talk between kernel and userspace in the first place. You shouldn't, we have __u32 / __u64 / etc for that.

Satyam

#5 lore raw
Quoted 8 lines · Satyam Sharma
Quoted 8 lines
Quoted 3 lines

On a 32-bit arch "unsigned long" is 32-bit and pointers are 32-bit.

On a 64-bit archi "unsigned long" is 64-bit and pointers are 64-bit.

So with 32 bit userspace "unsigned long long" is the type to use when talking to a 64-bit kernel; and with pure 64-bit code, it's enough to write "unsigned long".

Quoted 6 lines
Quoted 2 lines

I'm fairly sure that's the root cause of the pain I recall here; but I'd have to run experiments again to verify that.

I suspect the root cause of the pain was that you used "int" or "long" to talk between kernel and userspace in the first place. You shouldn't, we have __u32 / __u64 / etc for that.

Nope; the relevant code was always with "__u64". The issue was warning when turning that into a __user pointer.

Quoted 4 lines

The reason I ask is that gcc will also complain (understandably so) with "warning: cast from pointer to integer of different size" i.e. even if it's a conversion from smaller size to greater size, and not really a case of truncation.

ISTR the warning was the other way around: about "cast from integer
to pointer of a different size". The __u64 came from userspace and the kernel pointer was only 32 bits. Not really truncation, but GCC could not know that directly ... ergo the extra non-pointer cast.

author
#6 lore raw
Quoted 4 lines · David Brownell

ISTR the warning was the other way around: about "cast from integer
to pointer of a different size". The __u64 came from userspace and the kernel pointer was only 32 bits. Not really truncation, but GCC could not know that directly ... ergo the extra non-pointer cast.

And? Cast to integer type with the size equal to that of pointer. unsigned long is just that on all supported targets.

More interesting question is whether you want an error returned when pointers are 32bit and value doesn't fit into that...

#7 lore raw
Quoted 9 lines · Al Viro
Quoted 4 lines · David Brownell

ISTR the warning was the other way around: about "cast from integer
to pointer of a different size". The __u64 came from userspace and the kernel pointer was only 32 bits. Not really truncation, but GCC could not know that directly ... ergo the extra non-pointer cast.

And? Cast to integer type with the size equal to that of pointer. unsigned long is just that on all supported targets.

Some tool kept warning about that. Presumably then-current sparse. I've certainly heard the conventional "unsigned long fits pointers" wisdom, but tools disagreed. (Does ANSI C guarantee that? I'd think not, or uintptr_t would not be needed.)

And ptrdiff_t was the closest relevant data type that passed both gcc and sparse, since uintptr_t didn't previously exist everywhere.

Quoted 2 lines

More interesting question is whether you want an error returned when pointers are 32bit and value doesn't fit into that...

Either access_ok() or copy_from_user() reports an error if the pointer part of that u64 (N LSBs) is bad.

As a general policy, I think the other part is undefined and irrelevant to the kernel ... it's a kind of explicit padding, and padding isn't valdated. (At most it's zeroed to prevent a covert channel, but that's not relevent here.)

- Dave

author
#8 lore raw
In reply to Anton Altaparmakov
Quoted 13 lines · Anton Altaparmakov

Hi,

Quoted 10 lines · David Brownell
Quoted 4 lines · Al Viro

is wrong; for one thing, it's a bad C (it's what uintptr_t is for; in general
we are not even promised that ptrdiff_t is large enough to hold a pointer,

ISTR we don't *have* a uintptr_t on all architectures, or that would be the appropriate thing to use in these 32/64 bit ABI scenarios.

Quoted 8 lines
Quoted 3 lines
Quoted 1 line

Use unsigned long or uintptr_t instead.

I suspect you mean "unsigned long long"...

No he doesn't. "unsigned long" is guaranteed to be large enough to hold a pointer (at least on Linux anyway).

On a 32-bit arch "unsigned long" is 32-bit and pointers are 32-bit.

... while unsigned long long is 64bit, which is definitely not what one wants. For sparse it's "unsigned long is special".

FWIW, this patch puts it in linux/types.h as unsigned long. Eventually we might want to switch explicit casts to/from unsigned long in such contexts to uintptr_t, but for now we can't start complaining about unsigned long - too many places are using it. I'll see what can be done to get sane assistance from sparse in that kind of work...

#9 lore raw
In reply to Al Viro
Quoted 9 lines · Al Viro

Use of ptrdiff_t in

- if (!access_ok(VERIFY_WRITE, u_tmp->rx_buf,
u_tmp->len))

(quoted diff) +2 -0
+                       if (!access_ok(VERIFY_WRITE, (u8 __user *)
+                                               (ptrdiff_t) u_tmp-
Quoted 1 line

rx_buf,

+ u_tmp->len))

Quoted 8 lines

is wrong; for one thing, it's a bad C (it's what uintptr_t is for; in general we are not even promised that ptrdiff_t is large enough to hold a pointer, just enough to hold a difference between two pointers within the same object). For another, it confuses the fsck out of sparse.

Use unsigned long or uintptr_t instead. There are several places misusing ptrdiff_t (one - in a very odd way; WTF did that cast to __user ptrdiff_t
in ntfs expect to happen, anyway?) Fixed...

My current NTFS code has this already fixed. I used unsigned long instead of uintptr_t though... Feel free to apply this though as I have no idea when I will have time/permission to push an update upstream...

And what the cast was doing I can't remember. I may well have just copied it from the VFS or I was perhaps trying to silence a warning and this happened to work... But yes I noticed that more recent versions of sparse complained about it and fixed it with an unsigned long cast.

Best regards,

Anton

Quoted 157 lines
Signed-off-by Al Viro <[email protected]>

---

drivers/scsi/aacraid/commctrl.c +1 -1
index 72b0393..1e6d7a9 100644
@@ -391,7 +391,7 @@ static int close_getadapter_fib(struct aac_dev  
 * dev, void __user *arg)
 		/*
 		 *	Extract the fibctx from the input parameters
 		 */
-		if (fibctx->unique == (u32)(ptrdiff_t)arg) /* We found a winner */
+		if (fibctx->unique == (u32)(uintptr_t)arg) /* We found a winner */
 			break;
 		entry = entry->next;
drivers/scsi/aacraid/commctrl.c +3 -3
 		fibctx = NULL;

@@ -590,7 +590,7 @@ static int aac_send_raw_srb(struct aac_dev*  
 dev, void __user * arg)
 				}
 				addr = (u64)upsg->sg[i].addr[0];
 				addr += ((u64)upsg->sg[i].addr[1]) << 32;
-				sg_user[i] = (void __user *)(ptrdiff_t)addr;
+				sg_user[i] = (void __user *)(uintptr_t)addr;
 				sg_list[i] = p; // save so we can clean up later
 				sg_indx = i;

@@ -633,7 +633,7 @@ static int aac_send_raw_srb(struct aac_dev*  
 dev, void __user * arg)
 					rcode = -ENOMEM;
 					goto cleanup;
 				}
-				sg_user[i] = (void __user *)(ptrdiff_t)usg->sg[i].addr;
+				sg_user[i] = (void __user *)(uintptr_t)usg->sg[i].addr;
 				sg_list[i] = p; // save so we can clean up later
 				sg_indx = i;

@@ -664,7 +664,7 @@ static int aac_send_raw_srb(struct aac_dev*  
 dev, void __user * arg)
 		if (actual_fibsize64 == fibsize) {
 			struct user_sgmap64* usg = (struct user_sgmap64 *)upsg;
 			for (i = 0; i < upsg->count; i++) {
-				u64 addr;
+				uintptr_t addr;
 				void* p;
 				/* Does this really need to be GFP_DMA? */
drivers/scsi/aacraid/commctrl.c +2 -2
 				p = kmalloc(usg->sg[i].count,GFP_KERNEL|__GFP_DMA);

@@ -676,7 +676,7 @@ static int aac_send_raw_srb(struct aac_dev*  
 dev, void __user * arg)
 				}
 				addr = (u64)usg->sg[i].addr[0];
 				addr += ((u64)usg->sg[i].addr[1]) << 32;
-				sg_user[i] = (void __user *)(ptrdiff_t)addr;
+				sg_user[i] = (void __user *)addr;
 				sg_list[i] = p; // save so we can clean up later
 				sg_indx = i;

@@ -704,7 +704,7 @@ static int aac_send_raw_srb(struct aac_dev*  
 dev, void __user * arg)
 					rcode = -ENOMEM;
 					goto cleanup;
 				}
-				sg_user[i] = (void __user *)(ptrdiff_t)upsg->sg[i].addr;
+				sg_user[i] = (void __user *)(uintptr_t)upsg->sg[i].addr;
 				sg_list[i] = p; // save so we can clean up later
 				sg_indx = i;
drivers/scsi/aacraid/comminit.c +0 -0
index 3009ad8..8736813 100644
@@ -110,7 +110,7 @@ static int aac_alloc_comm(struct aac_dev *dev,

void **commaddr, unsigned long co

drivers/scsi/aacraid/comminit.c +1 -1
 	/*
 	 *	Align the beginning of Headers to commalign
 	 */
-	align = (commalign - ((ptrdiff_t)(base) & (commalign - 1)));
+	align = (commalign - ((uintptr_t)(base) & (commalign - 1)));
 	base = base + align;
 	phys = phys + align;
 	/*
drivers/scsi/aacraid/dpcsup.c +1 -1
index fcd25f7..e6032ff 100644
@@ -254,7 +254,7 @@ unsigned int aac_intr_normal(struct aac_dev *  
 dev, u32 Index)
 			kfree (fib);
 			return 1;
 		}
-		memcpy(hw_fib, (struct hw_fib *)(((ptrdiff_t)(dev->regs.sa)) +
+		memcpy(hw_fib, (struct hw_fib *)(((uintptr_t)(dev->regs.sa)) +
 		  (index & ~0x00000002L)), sizeof(struct hw_fib));
 		INIT_LIST_HEAD(&fib->fiblink);
drivers/scsi/aacraid/dpcsup.c +0 -0
 		fib->type = FSAFS_NTC_FIB_CONTEXT;
drivers/spi/spidev.c +2 -2
index c55459c..b3518ca 100644
@@ -184,14 +184,14 @@ static int spidev_message(struct spidev_data  
 *spidev,
 		if (u_tmp->rx_buf) {
 			k_tmp->rx_buf = buf;
 			if (!access_ok(VERIFY_WRITE, (u8 __user *)
-						(ptrdiff_t) u_tmp->rx_buf,
+						(uintptr_t) u_tmp->rx_buf,
 						u_tmp->len))
 				goto done;
 		}
 		if (u_tmp->tx_buf) {
 			k_tmp->tx_buf = buf;
 			if (copy_from_user(buf, (const u8 __user *)
-						(ptrdiff_t) u_tmp->tx_buf,
+						(uintptr_t) u_tmp->tx_buf,
 					u_tmp->len))
 				goto done;
drivers/spi/spidev.c +1 -1
 		}

@@ -224,7 +224,7 @@ static int spidev_message(struct spidev_data  
 *spidev,
 	for (n = n_xfers, u_tmp = u_xfers; n; n--, u_tmp++) {
 		if (u_tmp->rx_buf) {
 			if (__copy_to_user((u8 __user *)
-					(ptrdiff_t) u_tmp->rx_buf, buf,
+					(uintptr_t) u_tmp->rx_buf, buf,
 					u_tmp->len)) {
 				status = -EFAULT;
drivers/spi/spidev.c +0 -0
 				goto done;
fs/ntfs/file.c +0 -0
index ffcc504..a9103ce 100644
@@ -362,7 +362,7 @@ static inline void ntfs_fault_in_pages_readable 
 (const char __user *uaddr,
 	volatile char c;
 
 	/* Set @end to the first byte outside the last page we care

about. */

fs/ntfs/file.c +0 -1
-	end = (const char __user*)PAGE_ALIGN((ptrdiff_t __user)uaddr +

bytes);

fs/ntfs/file.c +1 -0
+	end = (const char __user*)PAGE_ALIGN((uintptr_t)uaddr + bytes);
 
 	while (!__get_user(c, uaddr) && (uaddr += PAGE_SIZE, uaddr < end))
 		;
include/linux/types.h +2 -1
index 0351bf2..efdec19 100644
@@ -40,6 +40,8 @@ typedef __kernel_gid32_t	gid_t;
 typedef __kernel_uid16_t        uid16_t;
 typedef __kernel_gid16_t        gid16_t;
 
+typedef unsigned long		uintptr_t;
+
 #ifdef CONFIG_UID16
 /* This is defined by include/asm-{arch}/posix_types.h */
 typedef __kernel_old_uid_t	old_uid_t;
-
Quoted 4 lines

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Best regards,

Anton

Signature · 3 lines
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/
author
#10 lore raw
Quoted 3 lines · Anton Altaparmakov

And what the cast was doing I can't remember. I may well have just copied it from the VFS or I was perhaps trying to silence a warning and this happened to work...

... due to sparse bug (it miscalculated address space of pointed to, picking top-level qualifiers for some reason; __user pointer to char and pointer to __user char gave the same result and so did __user long). That cast still made no sense...

#11 lore raw
Quoted 10 lines · Al Viro
Quoted 3 lines · Anton Altaparmakov

And what the cast was doing I can't remember. I may well have just copied it from the VFS or I was perhaps trying to silence a warning and this happened to work...

... due to sparse bug (it miscalculated address space of pointed to, picking top-level qualifiers for some reason; __user pointer to char and pointer to __user char gave the same result and so did __user long). That cast still made no sense...

I could believe that all came from sparse bugs. Fixed now?

#12 lore raw
In reply to Al Viro
Quoted 5 lines · Al Viro

Use unsigned long or uintptr_t instead.  There are several places misusing ptrdiff_t (one - in a very odd way; WTF did that cast to __user ptrdiff_t in ntfs expect to happen, anyway?)   Fixed...

Signed-off-by Al Viro <[email protected]>

By the way: ACK. Glad there's now always a "uintptr_t".