Re: IA64 ino_t incorrectly sized?

From: Nathan Scott <nathans_at_sgi.com>
Date: 2003-10-15 11:25:04
hi David,

[ Below are updated patches.  I've added Christoph and Andrew
onto the discussion because the patches add __ia64__ ifdef's
into core kernel headers, and they've shown me cleaner ways
to do this sort of thing in the past. ]

On Wed, Oct 08, 2003 at 06:57:13PM -0700, David Mosberger wrote:
> >>>>> On Thu, 9 Oct 2003 11:25:58 +1000, Nathan Scott <nathans@sgi.com> said:
>   Nathan> e.g. the ia64 include/asm-ia64/stat.h uses long for the
>   Nathan> inode number field, and the struct inode i_ino is a long
>   Nathan> also.  So, for the most part everything works as is - there
>   Nathan> are corner cases (i.e. getdents) that this fixes though, as
>   Nathan> I explained.  It does not change any external interfaces (if
>   Nathan> it did they'd be broken already, but I haven't seen any
>   Nathan> evidence of that in my testing).
> 
> Are you saying that you reviewed the entire syscall interface and did
> a reasonable review of anything else (e.g., /proc) that might expose
> ino_t in one way or another?

So, my brave assertion that no code would be exporting a type
called "__kernel_ino_t" turns out to be incorrect (/me hangs
head in shame) as there are two cases I see where this type
is exported to userspace.

Here's some more details about what I've reviewed and found.
If I've overlooked any areas let me know.

system call interface -- I examined the 2.4 IA64 system call
table and each of the structures passed across it in detail.
This revealed that the ustat and NFS system calls pass around
binary structures with __kernel_ino_t fields (see my updated
patches).  I then diff'd the 2.4 and 2.6 asm-ia64/unistd.h
and reviewed each of the new syscalls - there are no new 2.6
interfaces that deal with an ino_t.

procfs interfaces -- the one procfs interface I found dealing
with inode numbers is the /proc/PID/maps file.  In 2.4, this is
done in fs/proc/array.c::proc_pid_maps_get_line and in 2.6 its
done in fs/proc/task_mmu.c::show_map.  In both places, the ino
is copied out of (struct inode)->i_ino into a local unsigned
long and then formatted via "%lu".  Hence, ino_t doesn't come
into the picture here and we are not exposed to __kernel_ino_t
changes in either kernel.

ioctl interfaces -- there are several places in arch/ia64/ia32/
ia32_ioctl.c and sys_ia32.c where we copy an ino_t into an
unsigned int...
	sys_ia32.c::filldir32
	sys_ia32.c::fillonedir32
	ia32_ioctl.c::put_dirent32

These would change from copying 'unsigned int'->'unsigned int'
into 'unsigned long'->'unsigned int'.  It turns out that this
is already being done in other places, so I think its OK; e.g.
sys_ia32.c::putstat copies a kbuf->st_ino (unsigned long, from
the IA64 version of struct stat) into an ubuf->st_ino (uint).

From reviewing the code with Keith Owens, it looks like this
copies the first 4 bytes (sizeof the specified field in the
user buffer) and we've "got lucky" here due to our endianness
(otherwise we would get all zeroes).  Not sure if this was
by luck or by design, but obviously it works as is.


Anyway, here's the two patches (2.6 & 2.4).  The 2.4 version
no longer applies cleanly to 2.6 due to other kernel changes,
and the 2.6 kernel deprecates one of the 2.4 NFS structures.

I'd appreciate any suggestions about a cleaner way to do the
common types.h and nfsd/syscall.h header changes.

cheers.

-- 
Nathan

-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Received on Tue Oct 14 21:29:13 2003

This archive was generated by hypermail 2.1.8 : 2005-08-02 09:20:19 EST