[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] canonicalize: add support for not resolving symlinks
From: |
Jim Meyering |
Subject: |
Re: [PATCH] canonicalize: add support for not resolving symlinks |
Date: |
Fri, 30 Dec 2011 16:07:12 +0100 |
Pádraig Brady wrote:
> On 12/30/2011 01:49 PM, Eric Blake wrote:
>> On 12/30/2011 04:04 AM, Jim Meyering wrote:
>>>> - if (lstat (rname, &st) != 0)
>>>> + if ((logical?stat:lstat) (rname, &st) != 0)
>>>
>>> Please add spaces around operators:
>>>
>>> if ((logical ? stat : lstat) (rname, &st) != 0)
>>
>> Better yet, don't write this as a function pointer conditional, as the
>> gnulib replacement for stat on some platforms has to be a function-like
>> macro (no thanks to 'struct stat'). Using a function pointer
>> conditional risks missing the gnulib macro for stat, and calling the
>> underlying system stat() instead of the intended rpl_stat(), for broken
>> behavior. You have to use:
>>
>> if (logical ? stat (rname, &st) : lstat (rname, &st)) != 0)
>>
>
> Ouch. Good catch.
> I'll need to fix in a follow up patch.
> I'll need to adjust such uses in coreutils too.
Oh! I'd just finished checking gnulib for other instances,
then, after seeing your message, did this in coreutils:
$ git grep -E '\<l?stat *: *l?stat\>'
src/ln.c: (logical?stat:lstat) (source, &source_stats)
src/stat.c: (follow_links?stat:lstat) (filename, &statbug)
src/touch.c: /* Don't use (no_dereference?lstat:stat) (args), since stat
Here's a proposed addition to gnulib's maint.mk:
>From 24b71909a0b4022aeeea7fa74f250afa243e8d7a Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Fri, 30 Dec 2011 16:06:33 +0100
Subject: [PATCH] maint.mk: add a syntax check for subtle stat/lstat usage
problem
* top/maint.mk (sc_stat_lstat_without_open_paren): New rule.
Motivated by this exchange:
http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/29491/focus=29496
---
ChangeLog | 5 +++++
top/maint.mk | 10 ++++++++++
2 files changed, 15 insertions(+), 0 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index e61be47..e2e1567 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
2011-12-30 Jim Meyering <address@hidden>
+ maint.mk: add a syntax check for subtle stat/lstat usage problem
+ * top/maint.mk (sc_stat_lstat_without_open_paren): New rule.
+ Motivated by this exchange:
+ http://thread.gmane.org/gmane.comp.lib.gnulib.bugs/29491/focus=29496
+
gitlog-to-changelog: remove a little duplication
* build-aux/gitlog-to-changelog (main): Grep @lines once,
rather than twice.
diff --git a/top/maint.mk b/top/maint.mk
index e4efb5f..9386a61 100644
--- a/top/maint.mk
+++ b/top/maint.mk
@@ -1185,6 +1185,16 @@ sc_prohibit_path_max_allocation:
halt='Avoid stack allocations of size PATH_MAX' \
$(_sc_search_regexp)
+# Use stat or lstat only followed by an opening parenthesis, i.e.,
+# Don't do this: (cond ? lstat : stat) (....
+# Do this instead: (cond ? lstat (...) : stat (...))
+# Otherwise, they may fail to resolve (via a macro) to a desired
+# replacement function.
+sc_stat_lstat_without_open_paren:
+ @prohibit='\<l?stat *: *l?stat\>'
+ halt='don'\''t use stat or lstat without an open parenthesis' \
+ $(_sc_search_regexp)
+
sc_vulnerable_makefile_CVE-2009-4029:
@prohibit='perm -777 -exec chmod a\+rwx|chmod 777 \$$\(distdir\)' \
in_files=$$(find $(srcdir) -name Makefile.in) \
--
1.7.8.1.391.g2c2ad
- [PATCH] canonicalize: add support for not resolving symlinks, Pádraig Brady, 2011/12/29
- Re: [PATCH] canonicalize: add support for not resolving symlinks, Paul Eggert, 2011/12/30
- Re: [PATCH] canonicalize: add support for not resolving symlinks, Eric Blake, 2011/12/30
- Re: [PATCH] canonicalize: add support for not resolving symlinks, Paul Eggert, 2011/12/30
- Re: [PATCH] canonicalize: add support for not resolving symlinks, Eric Blake, 2011/12/31