This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: FD_SET and FORTIFY_SOURCE
- From: KOSAKI Motohiro <kosaki dot motohiro at gmail dot com>
- To: Carlos O'Donell <carlos at redhat dot com>
- Cc: KOSAKI Motohiro <kosaki dot motohiro at gmail dot com>, libc-alpha <libc-alpha at sourceware dot org>
- Date: Mon, 25 Mar 2013 14:41:08 -0400
- Subject: Re: FD_SET and FORTIFY_SOURCE
- References: <512F0CC6 dot 8080302 at redhat dot com> <20130228134139 dot GL20323 at brightrain dot aerifal dot cx> <512F84B2 dot 1000501 at redhat dot com> <512FCFD0 dot 5050108 at gmail dot com> <CAHGf_=pYJKnSbMu-N0_OLg7X+oNBa3527BbOXLeuiWzEZ1MPAg at mail dot gmail dot com> <515083A9 dot 5050407 at redhat dot com> <51508864 dot 80706 at gmail dot com> <51508DB5 dot 40606 at redhat dot com>
(3/25/13 1:47 PM), Carlos O'Donell wrote:
> On 03/25/2013 01:24 PM, KOSAKI Motohiro wrote:
>> (3/25/13 1:04 PM), Carlos O'Donell wrote:
>>> On 03/24/2013 04:17 PM, KOSAKI Motohiro wrote:
>>>> 1. only turn on __FD_ELT check when running on hurd.
>>>> 2. only turn on __FD_ELT check when defined some specific macro. (e.g.
>>>> likes darwin,
>>>> but disable by default)
>>>> 2-2. make FORTIFY_SOURCE variant and check POSIX compliance if enabled.
>>>> 3. provide select_large_fdset() likes solaris. (I strongly don't
>>>> recommend. all application
>>>> need to modify and recompilation)
>>>>
>>>> What do you think?
>>>
>>> The value of __FD_SETSIZE is part of the ABI and can't be changed.
>>
>> ??
>> We never discuss to change __FD_SETSIZE. A lot of UNIX support >FD_SETSIZE,
>> but didn't change FD_SETSIZE.
>
> Thank you for the clarification, you want to change the macros to simply
> avoid checking for the size and let the user manage it.
>
> The same problem still applies. If you pass this set to another library
> compiled with the old macros it will fail.
>
> How do you avoid that breakage without recompiling all dependent
> applications?
>
> Your change would be an API breakage?
It depned on what breakage is. The api was broken by following commit.
So, reverting might be breakage and might be just _fix_.
commit a0f33f996f7986dbf37631a4577f8565b42df29e
Author: Ulrich Drepper <drepper@gmail.com>
Date: Thu Sep 8 19:48:47 2011 -0400
Add range checking for FD_SET, FD_CLR, and FD_ISSET
But, you are right. interoperability is also important. So, I'd suggest
to apply following incremental patch w/ my previous one.
diff --git a/debug/fdelt_chk.c b/debug/fdelt_chk.c
index d149476..e14ec6e 100644
--- a/debug/fdelt_chk.c
+++ b/debug/fdelt_chk.c
@@ -21,9 +21,6 @@
long int
__fdelt_chk (long int d)
{
- if (d < 0 || d >= FD_SETSIZE)
- __chk_fail ();
-
return d / __NFDBITS;
}
I mean, old application still may call __fdelt_chk(), but it doesn't make
program abort.
>>> Alternatively rewrite using poll/epoll.
>>
>> As far as I know, all of moder scripting language expose select interface to scripting
>> and poll has different semantics against select(2). That makes no sense.
>>
>> One of the way is, to avoid to use FD_SET and other FD_* family likes perl.
>> perf enforce users to make fd bitmap by themself (by using vec()).
>>
>> http://perldoc.perl.org/functions/select.html
>>
>> But, it is extream ugly and useless. I don't think this makes a lot of sense.
>
> Given that FD_SET that checks for <= 1024 is already in use,
Who? I really doubt anybody uses current macro meangfullly.
> you can't
> change the interface without interoperability issues between libraries.
>
> New macro?
New macro doesn't solve anything. New macro mean endorse to recompile. But when
recompiling is acceptable, we can just change and fix current interface.
FD_SET is already in use from multiple applications w/ howmany(). again we don't
talk about new feature at all. I hope you also skimm debian code search or something else.
I would like to suggest _CHECK_FD_SETSIZE_STRICTLY new macro and enable range check only when
_CHECK_FD_SETSIZE_STRICTLY=1 and _FORTIFY_SOURCE=2. That doesn't break any existing applications.
likes below.
commit ee72234de4308a6bff1c970cd7624ab0da969739
Author: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
Date: Sun Mar 24 21:55:40 2013 -0400
[PATCH] enable FD_SETSIZE range check only when defined _CHECK_FD_SIZE_STRICTLY=1
Signed-off-by: KOSAKI Motohiro <kosaki.motohiro@gmail.com>
diff --git a/ChangeLog b/ChangeLog
index e7991c9..ef9d4f3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2013-03-24 KOSAKI Motohiro <kosaki.motohiro@gmail.com>
+
+ * misc/sys/select.h: Don't enable FD_* argument check
+ when running on Linux.
+
2013-01-09 Anton Blanchard <anton@samba.org>
* sysdeps/unix/sysv/linux/powerpc/sched_getcpu.c: New file.
diff --git a/debug/fdelt_chk.c b/debug/fdelt_chk.c
index d149476..334bde3 100644
--- a/debug/fdelt_chk.c
+++ b/debug/fdelt_chk.c
@@ -17,13 +17,22 @@
#include <sys/select.h>
+/* never be removed. this is used from old applications. */
+long int
+__fdelt_chk_nocheck (long int d)
+{
+ return d / __NFDBITS;
+}
+strong_alias (__fdelt_chk_nocheck, __fdelt_chk)
+strong_alias (__fdelt_chk, __fdelt_warn)
+
long int
-__fdelt_chk (long int d)
+__fdelt_check (long int d)
{
if (d < 0 || d >= FD_SETSIZE)
__chk_fail ();
return d / __NFDBITS;
}
-strong_alias (__fdelt_chk, __fdelt_warn)
+strong_alias (__fdelt_check, __fdelt_warning)
diff --git a/misc/bits/select2.h b/misc/bits/select2.h
index 03558c9..7fe7b66 100644
--- a/misc/bits/select2.h
+++ b/misc/bits/select2.h
@@ -21,8 +21,8 @@
#endif
/* Helper functions to issue warnings and errors when needed. */
-extern long int __fdelt_chk (long int __d);
-extern long int __fdelt_warn (long int __d)
+extern long int __fdelt_check (long int __d);
+extern long int __fdelt_warning (long int __d)
__warnattr ("bit outside of fd_set selected");
#undef __FD_ELT
#define __FD_ELT(d) ¥
@@ -31,5 +31,5 @@ extern long int __fdelt_warn (long int __d)
(__builtin_constant_p (__d) ¥
? (0 <= __d && __d < __FD_SETSIZE ¥
? (__d / __NFDBITS) ¥
- : __fdelt_warn (__d)) ¥
- : __fdelt_chk (__d)); })
+ : __fdelt_warning (__d)) ¥
+ : __fdelt_check (__d)); })
diff --git a/misc/sys/select.h b/misc/sys/select.h
index 21351fe..b748f61 100644
--- a/misc/sys/select.h
+++ b/misc/sys/select.h
@@ -124,7 +124,7 @@ extern int pselect (int __nfds, fd_set *__restrict __readfds,
/* Define some inlines helping to catch common problems. */
-#if __USE_FORTIFY_LEVEL > 0 && defined __GNUC__
+#if __USE_FORTIFY_LEVEL > 0 && defined __GNUC__ && _CHECK_FD_SIZE_STRICTLY > 0
# include <bits/select2.h>
#endif