NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/50131: botched compat for lfs syscalls



>Number:         50131
>Category:       kern
>Synopsis:       botched compat for lfs syscalls
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Aug 06 05:15:00 +0000 2015
>Originator:     David A. Holland
>Release:        NetBSD 7.99.20 (20150803)
>Organization:
>Environment:
System: NetBSD macaran 7.99.20 NetBSD 7.99.20 (MACARAN) #30: Mon Jul 27 20:25:15 EDT 2015  dholland@macaran:/usr/src/sys/arch/amd64/compile/MACARAN amd64
Architecture: x86_64
Machine: amd64
>Description:

Back many years ago, as part of other changes, the LFS cleaner ops
were moved from being syscalls to being fcntls. This was done at the
same time as a change to the BLOCK_INFO structure (part of the
interface of lfs_bmapv and lfs_markv) and the system calls were left
in place for compatibility with old cleaner binaries.

Only trouble is, it wasn't done right.

The immediate consequences of this are limited by the fact that only
lfs_cleanerd performs these operations; any other program calling them
is wrong and can be arbitrarily broken; and since the changes back
when, the cleaner has only used the fcntl interface. Therefore, only
old cleaners (at this point, *really* old cleaners) will ever issue
these calls, and the code in place handles that.

However, if someone were to recompile the old cleaner (ignoring for
the moment that it wouldn't build because of other changes) they would
find it calling syscalls expecting the old BLOCK_INFO but using the
new BLOCK_INFO. This is wrong and it should all really be cleaned up.

There is no need to make current versions of the lfs syscalls, as
nothing current uses them. However, this occasions some messy compat
phenomena, especially in libc.

>How-To-Repeat:

Code reading.

>Fix:

Things that need to be done:

1. There is code in lfs_syscalls.c for versions of of sys_lfs_bmapv
and sys_lfs_markv that use the current BLOCK_INFO. This code is not
compiled, being under an ifdef USE_64BIT_SYSCALLS that is not settable
anywhere. This code should be removed.

2. The lfs_bmapv and lfs_markv syscalls should be marked COMPAT_15
(yes, I said many years ago) in syscalls.master. Do not create new
non-compat ones. This requires renaming the sys_lfs_bmapv and
sys_lfs_markv entry points in lfs_syscalls.c, and updating the
syscall_package in lfs_vfsops.c.

3. The sys_lfs_bmapv and sys_lfs_markv entry points in lfs_syscalls.c
should be wrapped in #ifdef COMPAT_15.

4. Now the libc syscall stubs need to be hacked; as there will no
longer be a SYS_lfs_bmapv or SYS_lfs_markv, we need to generate
syscall stubs where the numbers are SYS_compat_15_lfs_bmapv/markv but
the symbols are just lfs_bmapv/lfs_bmarkv. This is possible with the
macros in SYS.h, but I think we may need custom logic in
libc/sys/Makefile.inc to generate suitable .S files.

5. These .S files should also use ERROR_REFERENCES to prevent any new
binaries being linked against them. (And they should be noted in
libc/shlib_version for removal at libc bump time.)

6. There do not appear to be userlevel declarations for the lfs_bmapv
or lfs_markv syscalls, which is good.

7. The lfs_segclean syscall should get the same treatment, both in the
kernel and in libc.

8. The lfs_segwait syscall should too, but this is complicated by the
fact that it contained a timeval and therefore got COMPAT_50 code.
Which, for some reason that remains unclear (Christos, you remember?)
is disabled. Ultimately the 1.5-era entry point should be calling the
currently disabled compat code (which should maybe be moved back to
lfs_syscalls.c(*)) and the __lfs_segwait50 entry point can just return
ENOSYS. (And we can even reuse the syscall number later if we want, as
nothing should have ever called it.)

9. This will need to be test-built with and without both COMPAT_15 and
COMPAT_50 to make sure none of the chewing gum falls off.

10. If someone feels brave, try testing with a 1.5-era cleaner binary.

(*) While all the compat code should really live in sys/compat,
currently moving the bmapv/markv entry points there will be somewhat
problematic (I think) due to #include horrors, and I am wondering if
the reason the compat lfs_segwait code is #ifdef notyet is that it
doesn't build where it is...

Conversely, we could add three more syscalls for the cleaner ops using
the current form of the structures; this would avoid the nonstandard
compat situation in libc (tempting) but as these calls would never be
used it seems like a waste of syscall table space.

On the third hand, we could decree that running a cleaner from NetBSD
1.5 is a stupid thing to do and rip it all out entirely -- it hasn't
worked since the time_t change anyway because lfs_segwait is disabled.

I have a patch that covers points 1-3, but I ran out of steam when I
looked at the libc issues.

Also note: I am about to commit changes that version BLOCK_INFO again,
so we'll have BLOCK_INFO (current), BLOCK_INFO_70 (1.6 through 7.0)
and BLOCK_INFO_15 (<= 1.5). This does not affect the syscall entry
points at all (since they're written to expect only BLOCK_INFO_15) and
said changes have no effect at all on the above points. Neither do any
COMPAT_32 changes that might appear in the fcntl dispatching.



Home | Main Index | Thread Index | Old Index