[Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Jeff Layton]

When I originally added OFD lock support, we made the assumption that
userland would always pass in a struct flock64 for an OFD lock. It's
possible however for someone to build a binary without large file
support (aka LFS), which will call down into the kernel with the
standard F_OFD_* constants, but pass in a "legacy" struct flock.

My first thought was to patch glibc to cause a build break if anyone
tries to use OFD locks without LFS, but now I think it might be less
problematic to simply support OFD locks without LFS enabled.

The kernel handles this for classic POSIX locks with a separate set of
constants (postfixed with "64") to indicate that the incoming structure
is an LFS one. We can't do that here since the kernel already assumes
that the structure is an LFS one.

Instead, we can define a new set of constants that are postfixed with
"32" to indicate that the incoming structure is a non-LFS one. This
patch adds the kernel plumbing to handle that case. We'll also need a
small patch for glibc to make it to use these constants when LFS is
disabled.

In the event that someone builds a program with a new glibc, and runs it
on a kernel without support for F_OFD_*32 constants, they will just get
back EINVAL. That's preferable to the current situation which is
undefined behavior due to misinterpretation of the struct flock
argument.

Cc: stable@vger.kernel.org	# v3.15+
Reported-by: Cyril Hrubis <chrubis@suse.cz>
Signed-off-by: Jeff Layton <jlayton@redhat.com>
---
 fs/compat.c                      | 3 +++
 fs/fcntl.c                       | 4 +++-
 fs/locks.c                       | 4 +++-
 include/uapi/asm-generic/fcntl.h | 4 ++++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/fs/compat.c b/fs/compat.c
index be6e48b0a46c..a7e9640e9107 100644
--- a/fs/compat.c
+++ b/fs/compat.c
@@ -426,6 +426,9 @@ COMPAT_SYSCALL_DEFINE3(fcntl64, unsigned int, fd, unsigned int, cmd,
 	case F_GETLK:
 	case F_SETLK:
 	case F_SETLKW:
+	case F_OFD_GETLK32:
+	case F_OFD_SETLK32:
+	case F_OFD_SETLKW32:
 		ret = get_compat_flock(&f, compat_ptr(arg));
 		if (ret != 0)
 			break;
diff --git a/fs/fcntl.c b/fs/fcntl.c
index 350a2c8cfd28..71704aa11170 100644
--- a/fs/fcntl.c
+++ b/fs/fcntl.c
@@ -270,6 +270,7 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
 	/* 32-bit arches must use fcntl64() */
 	case F_OFD_GETLK:
 #endif
+	case F_OFD_GETLK32:
 	case F_GETLK:
 		err = fcntl_getlk(filp, cmd, (struct flock __user *) arg);
 		break;
@@ -278,7 +279,8 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
 	case F_OFD_SETLK:
 	case F_OFD_SETLKW:
 #endif
-		/* Fallthrough */
+	case F_OFD_SETLK32:
+	case F_OFD_SETLKW32:
 	case F_SETLK:
 	case F_SETLKW:
 		err = fcntl_setlk(fd, filp, cmd, (struct flock __user *) arg);
diff --git a/fs/locks.c b/fs/locks.c
index 7e428b78be07..4ffde68322de 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2065,7 +2065,7 @@ int fcntl_getlk(struct file *filp, unsigned int cmd, struct flock __user *l)
 	if (error)
 		goto out;
 
-	if (cmd == F_OFD_GETLK) {
+	if (cmd == F_OFD_GETLK || cmd == F_OFD_GETLK32) {
 		error = -EINVAL;
 		if (flock.l_pid != 0)
 			goto out;
@@ -2222,6 +2222,7 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
 	 */
 	switch (cmd) {
 	case F_OFD_SETLK:
+	case F_OFD_SETLK32:
 		error = -EINVAL;
 		if (flock.l_pid != 0)
 			goto out;
@@ -2231,6 +2232,7 @@ int fcntl_setlk(unsigned int fd, struct file *filp, unsigned int cmd,
 		file_lock->fl_owner = filp;
 		break;
 	case F_OFD_SETLKW:
+	case F_OFD_SETLKW32:
 		error = -EINVAL;
 		if (flock.l_pid != 0)
 			goto out;
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index e063effe0cc1..b407deee68e1 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -148,6 +148,10 @@
 #define F_OFD_SETLK	37
 #define F_OFD_SETLKW	38
 
+#define F_OFD_GETLK32	39
+#define F_OFD_SETLK32	40
+#define F_OFD_SETLKW32	41
+
 #define F_OWNER_TID	0
 #define F_OWNER_PID	1
 #define F_OWNER_PGRP	2
-- 
2.7.4

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Cyril Hrubis]

Hi!
> diff --git a/fs/fcntl.c b/fs/fcntl.c
> index 350a2c8cfd28..71704aa11170 100644
> --- a/fs/fcntl.c
> +++ b/fs/fcntl.c
> @@ -270,6 +270,7 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
>  	/* 32-bit arches must use fcntl64() */
>  	case F_OFD_GETLK:
>  #endif
> +	case F_OFD_GETLK32:
>  	case F_GETLK:
>  		err = fcntl_getlk(filp, cmd, (struct flock __user *) arg);
>  		break;
> @@ -278,7 +279,8 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
>  	case F_OFD_SETLK:
>  	case F_OFD_SETLKW:
>  #endif
> -		/* Fallthrough */
> +	case F_OFD_SETLK32:
> +	case F_OFD_SETLKW32:
>  	case F_SETLK:
>  	case F_SETLKW:
>  		err = fcntl_setlk(fd, filp, cmd, (struct flock __user *) arg);

Shouldn't we do #if BITS_PER_LONG == 32 around the newly added cases?

Since otherwise fcntl() with cmd F_OFD_SETLK32 would expect 64bit off_t
on 64 bit kernel. It will probably never be used that way, but I find it
quite confusing.

The rest looks good to me.

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Jeff Layton]

On Thu, 2016-08-18 at 15:24 +0200, Cyril Hrubis wrote:
> Hi!
> > 
> > diff --git a/fs/fcntl.c b/fs/fcntl.c
> > index 350a2c8cfd28..71704aa11170 100644
> > --- a/fs/fcntl.c
> > +++ b/fs/fcntl.c
> > @@ -270,6 +270,7 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
> > > >  	/* 32-bit arches must use fcntl64() */
> > > >  	case F_OFD_GETLK:
> >  #endif
> > > > +	case F_OFD_GETLK32:
> > > >  	case F_GETLK:
> > > >  		err = fcntl_getlk(filp, cmd, (struct flock __user *) arg);
> > > >  		break;
> > @@ -278,7 +279,8 @@ static long do_fcntl(int fd, unsigned int cmd, unsigned long arg,
> > > >  	case F_OFD_SETLK:
> > > >  	case F_OFD_SETLKW:
> >  #endif
> > > > -		/* Fallthrough */
> > > > +	case F_OFD_SETLK32:
> > > > +	case F_OFD_SETLKW32:
> > > >  	case F_SETLK:
> > > >  	case F_SETLKW:
> > > >  		err = fcntl_setlk(fd, filp, cmd, (struct flock __user *) arg);
> 
> Shouldn't we do #if BITS_PER_LONG == 32 around the newly added cases?
> 
> Since otherwise fcntl() with cmd F_OFD_SETLK32 would expect 64bit off_t
> on 64 bit kernel. It will probably never be used that way, but I find it
> quite confusing.
> 
> The rest looks good to me.
> 

No, 64 bit machines still need these for the compat syscall case.
Consider someone running a 32-bit, non-LFS binary on a 64-bit host.

Unfortunately, the way this has changed over the decades is just really
hard to follow. Eventually we ought to do a cleanup of this code to
make it simpler, but I'd really like this patch to be applicable to
stable kernels, so I think we ought to wait on that until later.

-- 
Jeff Layton <jlayton@redhat.com>

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Cyril Hrubis]

> > Shouldn't we do #if BITS_PER_LONG == 32 around the newly added cases?
> > 
> > Since otherwise fcntl() with cmd F_OFD_SETLK32 would expect 64bit off_t
> > on 64 bit kernel. It will probably never be used that way, but I find it
> > quite confusing.
> > 
> > The rest looks good to me.
> > 
> 
> No, 64 bit machines still need these for the compat syscall case.
> Consider someone running a 32-bit, non-LFS binary on a 64-bit host.

Ah, we call the sys_fcntl() with these from the compat code supposedly
so that it does all the checks we omit in the compat variant. Then it's
needed and confusing at the same time.

We do convert_fcntl_cmd() for the 64bit variants already, maybe we can
just add the 32bit variants to the switch there as well. I'm not sure if
it is worth of the code size increase though.

> Unfortunately, the way this has changed over the decades is just really
> hard to follow. Eventually we ought to do a cleanup of this code to
> make it simpler, but I'd really like this patch to be applicable to
> stable kernels, so I think we ought to wait on that until later.

I guess that this is fine for quick fix. Cleanup of the code would be
nice, it's quite a maze as it is.

-- 
Cyril Hrubis
chrubis@suse.cz

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Christoph Hellwig]

NAK.  People should stop using 32-bit off_t and friends yesterday.
It's a shame that glibc hasn't cought up with last century yet and
stopped providing the non-LFS APIs for newly compiled code, but 
we certainly should not bloat the kernel for the idiotic behavior.

In addition anyone is going to use a new Linux-only feature like the
OFS locks should be using LFS support anyway.

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Jeff Layton]

On Thu, 2016-08-18 at 19:05 +0200, Christoph Hellwig wrote:
> NAK.  People should stop using 32-bit off_t and friends yesterday.
> It's a shame that glibc hasn't cought up with last century yet and
> stopped providing the non-LFS APIs for newly compiled code, but 
> we certainly should not bloat the kernel for the idiotic behavior.
> 
> In addition anyone is going to use a new Linux-only feature like the
> OFS locks should be using LFS support anyway.

That was my original thinking, but several people seemed to think that
we should just go ahead and support it. TBH, I don't much care either
way, but we either need to support it properly, or ensure that trying
to use OFD locks in a non-LFS program fails to compile.

The only real concern I have here is whether limiting this to LFS
enabled programs might make it tougher to get this into POSIX. Would
the POSIX standards folks object to having an interface like this that
doesn't support non-LFS cases? I guess if that ever happens though,
then we can just widen the support at that point.

If this is the consensus, then we can go with something like the glibc
patch I sent yesterday, which disables the command definitions when LFS
is not in effect.

Thoughts?
-- 
Jeff Layton <jlayton@redhat.com>

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Christoph Hellwig]

On Thu, Aug 18, 2016 at 01:28:20PM -0400, Jeff Layton wrote:
> That was my original thinking, but several people seemed to think that
> we should just go ahead and support it. TBH, I don't much care either
> way, but we either need to support it properly, or ensure that trying
> to use OFD locks in a non-LFS program fails to compile.

Yes, that's what glibc folks should do for now given that they still
seem to refuse being draggred into the present.

> The only real concern I have here is whether limiting this to LFS
> enabled programs might make it tougher to get this into POSIX. Would
> the POSIX standards folks object to having an interface like this that
> doesn't support non-LFS cases? I guess if that ever happens though,
> then we can just widen the support at that point.

LFS is perfectly Posix compliant (as is non-LFS).  It's really just
a glibc (aka Linux) special to still support non-LFS modes.  4.4BSD
and decendants have made the switch to 64-bit off_t in 1994 and haven't
supported a non-LFS since.

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 1967 bytes --]

On 18 Aug 2016 19:31, Christoph Hellwig wrote:
> On Thu, Aug 18, 2016 at 01:28:20PM -0400, Jeff Layton wrote:
> > That was my original thinking, but several people seemed to think that
> > we should just go ahead and support it. TBH, I don't much care either
> > way, but we either need to support it properly, or ensure that trying
> > to use OFD locks in a non-LFS program fails to compile.
> 
> Yes, that's what glibc folks should do for now given that they still
> seem to refuse being draggred into the present.
> 
> > The only real concern I have here is whether limiting this to LFS
> > enabled programs might make it tougher to get this into POSIX. Would
> > the POSIX standards folks object to having an interface like this that
> > doesn't support non-LFS cases? I guess if that ever happens though,
> > then we can just widen the support at that point.
> 
> LFS is perfectly Posix compliant (as is non-LFS).  It's really just
> a glibc (aka Linux) special to still support non-LFS modes.  4.4BSD
> and decendants have made the switch to 64-bit off_t in 1994 and haven't
> supported a non-LFS since.

there's no need to be so dramatic here.  glibc didn't write the LFS logic
for fun, and hasn't maintained it out of laziness.  in fact, the code is
non-trivial to get right.  the trade offs were considered heavily, and not
breaking backwards compatibility was considered more important.  otherwise
we'd have a repeat of the libc4/libc5 (or gcc's libstdc++.so.5) breakage
where all binaries stop working.  BSD's can get away with it because their
entire modus operandi is that you get no backwards compatibility.

there has been discussion on the glibc lists for how we can move forward
*safely*, but it's not something to be taken lightly -- LFS defines will
implicitly change ABI structs all over the place.  in the mean time, let's
just drop the pointless inflammatory editorializing since it contributes
nothing.
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Christoph Hellwig]

On Thu, Aug 18, 2016 at 10:46:07AM -0700, Mike Frysinger wrote:
> there's no need to be so dramatic here.  glibc didn't write the LFS logic
> for fun, and hasn't maintained it out of laziness.  in fact, the code is
> non-trivial to get right.

It hasn't maintained it out of lazyness, but out of stupidity - it's been
20 years overdue to get rid of supporting non-LFS for _new code_.  Keeping
the old symbols around is perfectly fine.  And at least a few years
ago I could run FreeBSD 1.x (pre-4.4BSD) code on recent FreeBSD systems
with the right compat defines in the kernel build and the compat libraries
just fine, so it's not like it's an unsolved problem.

At the same time glibc lazuness has caused us Linux developers tons of
problems due to applications or even system programs using the wrong
APIs as they still are the default, including random errors due to "too large"
inode numbers or offset.

So yes, I'm pissed that this crap isn't sorted out and have all the reaons
to be "dramatic".

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Mike Frysinger]

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

On 18 Aug 2016 19:52, Christoph Hellwig wrote:
> On Thu, Aug 18, 2016 at 10:46:07AM -0700, Mike Frysinger wrote:
> > there's no need to be so dramatic here.  glibc didn't write the LFS logic
> > for fun, and hasn't maintained it out of laziness.  in fact, the code is
> > non-trivial to get right.
> 
> It hasn't maintained it out of lazyness, but out of stupidity - it's been
> 20 years overdue to get rid of supporting non-LFS for _new code_.

as i said, we've been discussing it of late, and it's a non-trivial problem.
"just make it the default" ignores the fact that LFS shows up in many places
and changes ABIs of downstream libs implicitly when they use impacted structs.

> Keeping
> the old symbols around is perfectly fine.  And at least a few years
> ago I could run FreeBSD 1.x (pre-4.4BSD) code on recent FreeBSD systems
> with the right compat defines in the kernel build and the compat libraries
> just fine, so it's not like it's an unsolved problem.

"and the compat libs" is a pretty key point.  of course if your lowest ABI
boundary is the kernel, things are much easier.  you can do the same thing
with libc5 today because the boundary is the Linux syscall ABI.  the point
is to *not* have to do that but keep using the same SONAMEs which does work
under Linux/glibc today, and generally is what the BSDs do not care about.

> At the same time glibc lazuness has caused us Linux developers tons of
> problems due to applications or even system programs using the wrong
> APIs as they still are the default, including random errors due to "too large"
> inode numbers or offset.

the APIs need to stick around regardless of what glibc does moving forward.  
existing binaries aren't going anywhere.  so if the compat syscals are broken,
then they're broken and need fixing.

> So yes, I'm pissed that this crap isn't sorted out and have all the reaons
> to be "dramatic".

which isn't terribly useful and is more likely to have people just ignore you
-mike

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Zack Weinberg]

On Thu, Aug 18, 2016 at 1:52 PM, Christoph Hellwig <hch@lst.de> wrote:
> On Thu, Aug 18, 2016 at 10:46:07AM -0700, Mike Frysinger wrote:
>> there's no need to be so dramatic here.  glibc didn't write the LFS logic
>> for fun, and hasn't maintained it out of laziness.  in fact, the code is
>> non-trivial to get right.
>
> It hasn't maintained it out of lazyness, but out of stupidity - it's been
> 20 years overdue to get rid of supporting non-LFS for _new code_.

Please say concretely what you mean by "get rid of supporting non-LFS
for new code".  I can think of only two possibilities, both with
severe negative side effects.

We could change the libc headers used on old-ILP32 ABIs so that
_FILE_OFFSET_BITS=64 is defined by default (matching the LP64-ABI
headers).  This would break the ABI of every shared library that
exports a structure (transitively) containing a field of type off_t,
ino_t, fsblkcnt_t, fsfilcnt_t, or rlim_t.  It would also break
non-LFS-safe programs upon recompilation, although they could still be
recompiled with -D_FILE_OFFSET_BITS=32.  This latter breakage could
well be silent, going unnoticed until someone actually tries to use
the program with a very large file -- and such bugs can and have been
security-critical.  So by me this is already a bad plan.

Or we could go even further and remove all modes but
_FILE_OFFSET_BITS=64 from the libc headers, breaking non-LFS-safe
programs upon recompilation with no quick fix; that seems like an even
worse plan.

But perhaps you have something else in mind?

zw

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 769 bytes --]

Zack Weinberg wrote:
> We could change the libc headers used on old-ILP32 ABIs so that
> _FILE_OFFSET_BITS=64 is defined by default (matching the LP64-ABI
> headers).  This would break the ABI of every shared library that
> exports a structure (transitively) containing a field of type off_t,
> ino_t, fsblkcnt_t, fsfilcnt_t, or rlim_t.

As I understand it, most (all important?) such libraries are already compiled 
with _FILE_OFFSET_BITS=64 anyway, so their ABIs wouldn't break.

How about if we start the transition by deprecating the use of 32-bit off_t in 
user or library code on platforms with 32-bit long? The attached patch plus lots 
of similar patches, say (this is just a sketch). Really, it's long since time 
that file offsets were 64 bits.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: deprecate-32-bit-off-t.patch --]
[-- Type: text/x-diff; name="deprecate-32-bit-off-t.patch", Size: 496 bytes --]

diff --git a/libio/stdio.h b/libio/stdio.h
index e37f901..a5f65e9 100644
--- a/libio/stdio.h
+++ b/libio/stdio.h
@@ -88,7 +88,11 @@ typedef _G_va_list va_list;
 #if defined __USE_UNIX98 || defined __USE_XOPEN2K
 # ifndef __off_t_defined
 # ifndef __USE_FILE_OFFSET64
+#  if !_SUPPRESS_FILE_OFFSET_DEPRECATION && __LONG_MAX__ < __LONG_LONG_MAX__
+typedef __off_t off_t __attribute_deprecated__;
+#  else
 typedef __off_t off_t;
+#  endif
 # else
 typedef __off64_t off_t;
 # endif

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Joseph Myers]

On Thu, 18 Aug 2016, Zack Weinberg wrote:

> We could change the libc headers used on old-ILP32 ABIs so that
> _FILE_OFFSET_BITS=64 is defined by default (matching the LP64-ABI
> headers).  This would break the ABI of every shared library that

This.  We have a patch 
<https://sourceware.org/ml/libc-alpha/2014-03/msg00290.html>, and we have 
an analysis <https://sourceware.org/ml/libc-alpha/2014-03/msg00351.html> 
providing some evidence, if not for a very large set of libraries, that 
_FILE_OFFSET_BITS=64 is the normal way GNU/Linux distributions build 
affected libraries.  We have, since then, _FILE_OFFSET_BITS=64 support for 
fts.  (It may be the case that such a patch would no longer build, or 
would cause linknamespace tests to fail because of bug 14106 - which would 
need addressing with new implementation-namespace symbols before such a 
patch could go in.)  But this requires someone to take the lead on getting 
all the prerequisites into glibc and establishing consensus for the change 
(possibly with a more detailed analysis of the use of _FILE_OFFSET_BITS=64 
to build affected libraries in GNU/Linux distributions).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Zack Weinberg]

On 08/18/2016 03:36 PM, Paul Eggert wrote:
> Zack Weinberg wrote:
>> We could change the libc headers used on old-ILP32 ABIs so that
>> _FILE_OFFSET_BITS=64 is defined by default (matching the LP64-ABI
>> headers).  This would break the ABI of every shared library that
>> exports a structure (transitively) containing a field of type off_t,
>> ino_t, fsblkcnt_t, fsfilcnt_t, or rlim_t.
> 
> As I understand it, most (all important?) such libraries are already
> compiled with _FILE_OFFSET_BITS=64 anyway, so their ABIs wouldn't break.

I'd feel a lot safer about changing the default if we had some way of
tagging object files and shared libraries so that _FILE_OFFSET_BITS=32
and _FILE_OFFSET_BITS=64 code could not be combined.  Do we already have
that?  It seems like something that could probably be done with existing
object-annotation goo, if we don't.

> How about if we start the transition by deprecating the use of 32-bit
> off_t in user or library code on platforms with 32-bit long? The
> attached patch plus lots of similar patches, say (this is just a
> sketch). Really, it's long since time that file offsets were 64 bits.

I'd support this patch (better with __attribute_deprecated_msg__() and
an explanation that it's not off_t that's deprecated, it's *32-bit* off_t).

zw

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Joseph Myers]

On Fri, 19 Aug 2016, Zack Weinberg wrote:

> I'd feel a lot safer about changing the default if we had some way of
> tagging object files and shared libraries so that _FILE_OFFSET_BITS=32
> and _FILE_OFFSET_BITS=64 code could not be combined.  Do we already have

But combining such files is fine if _FILE_OFFSET_BITS is not part of the 
ABI for those libraries (and there are plenty of libraries for which 
that's the case, even if they should be using _FILE_OFFSET_BITS=64 
internally).

My view is that changing the default is better then the status quo where 
distributions provide libraries compiled with a mixture of 
_FILE_OFFSET_BITS settings and users need to match the setting for a given 
library where it's part of the ABI.  The change should rapidly lead to 
very few people using _FILE_OFFSET_BITS=32 (but making that into just a 
glibc-internal "don't redirect functions in headers" mode for compiling 
the glibc compat versions of functions for old binaries, rather than 
supporting _FILE_OFFSET_BITS=32 for users, would be harder).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Zack Weinberg]

On Fri, Aug 19, 2016 at 11:02 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Fri, 19 Aug 2016, Zack Weinberg wrote:
>> I'd feel a lot safer about changing the default if we had some way of
>> tagging object files and shared libraries so that _FILE_OFFSET_BITS=32
>> and _FILE_OFFSET_BITS=64 code could not be combined.  Do we already have
>
> But combining such files is fine if _FILE_OFFSET_BITS is not part of the
> ABI for those libraries (and there are plenty of libraries for which
> that's the case, even if they should be using _FILE_OFFSET_BITS=64
> internally).

(I'm taking this as an implied statement that we *don't* have any such
tagging now.)

It's true that tagging would over-estimate incompatibility, but if
doing it reduces the risk of invisible breakage enough that we can
feel confident about changing the default, maybe we should do it
anyway.

(I'd still worry about invisible breakage of _applications_ due to
e.g. assuming sizeof(off_t) <= sizeof(size_t), but there's probably
nothing the _library_ can do about that.)

zw

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Joseph Myers]

On Fri, 19 Aug 2016, Zack Weinberg wrote:

> (I'd still worry about invisible breakage of _applications_ due to
> e.g. assuming sizeof(off_t) <= sizeof(size_t), but there's probably
> nothing the _library_ can do about that.)

We have the statement from the previous discussion that the change would 
break INN but distributions already dealt with LFS issues for it 
<https://sourceware.org/ml/libc-alpha/2014-03/msg00354.html>, but I expect 
that's the rare case, and not a significant argument against making the 
change.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Zack Weinberg]

On Fri, Aug 19, 2016 at 12:58 PM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Fri, 19 Aug 2016, Zack Weinberg wrote:
>
>> (I'd still worry about invisible breakage of _applications_ due to
>> e.g. assuming sizeof(off_t) <= sizeof(size_t), but there's probably
>> nothing the _library_ can do about that.)
>
> We have the statement from the previous discussion that the change would
> break INN but distributions already dealt with LFS issues for it
> <https://sourceware.org/ml/libc-alpha/2014-03/msg00354.html>, but I expect
> that's the rare case, and not a significant argument against making the
> change.

INN was on-disk data structures containing off_t quantities, wasn't
it?  I was thinking more like

   struct stat st;
   fstat(fd, &st);
   buf = malloc(st.st_size);
   read(fd, buf, st.st_size);

which is catastrophically wrong with 64-bit off_t *but only when
size_t is smaller*, i.e. this won't have been flushed out by testing
in an LP64 environment.  Neither gcc nor clang issues any diagnostic
for the truncation in the call to malloc, even at -std=c11 -Wall
-Wextra -pedantic, and nothing bad will happen at runtime unless the
program is actually supplied with a >2GB file.  So it's a lurking
at-least-crash-perhaps-exploit.

(You *can* get a warning with -Wconversion, but who uses that?  I
wonder if there's something glibc can do in its headers to make
warnings happen in this circumstance.  Nothing comes to mind.)

zw

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Florian Weimer]

* Christoph Hellwig:

> On Thu, Aug 18, 2016 at 01:28:20PM -0400, Jeff Layton wrote:
>> That was my original thinking, but several people seemed to think that
>> we should just go ahead and support it. TBH, I don't much care either
>> way, but we either need to support it properly, or ensure that trying
>> to use OFD locks in a non-LFS program fails to compile.
>
> Yes, that's what glibc folks should do for now given that they still
> seem to refuse being draggred into the present.

Your assumptions are wrong, at least for some (many?) of us.  32-bit
architectures are legacy; giving them a 64-bit off_t (or even time_t)
does not really change that.  A hard ABI transition is simply not
worth the effort.

Re: [Linux PATCH] fcntl: add new F_OFD_*32 constants and handle them appropriately

[Cyril Hrubis]

Hi!
> > Yes, that's what glibc folks should do for now given that they still
> > seem to refuse being draggred into the present.
> 
> Your assumptions are wrong, at least for some (many?) of us.  32-bit
> architectures are legacy; giving them a 64-bit off_t (or even time_t)
> does not really change that.  A hard ABI transition is simply not
> worth the effort.

Unfortunately there are still 32bit processors manufactured and sold
even these days, mainly for IOT though.

For instance Intel has released Quark (32bit i586 400Mhz CPU used in
Edison board) in 2014. First two batches of Raspberry Pi were 32bit
ARMv6/ARMv7, these are still sold today, etc.

So I would say that 32bit will stick with us for another ten years at
least.

-- 
Cyril Hrubis
chrubis@suse.cz