all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* unicode build: "comparison is always true" warnings
@ 2007-11-23  4:57 Mark A. Hershberger
  2007-11-23 15:09 ` Stefan Monnier
  0 siblings, 1 reply; 26+ messages in thread
From: Mark A. Hershberger @ 2007-11-23  4:57 UTC (permalink / raw)
  To: emacs-devel


I'm working on providing snapshots of the unicode branch and have been
getting a ton of these messages:

    warning: comparison is always true due to limited range of data type

Most are in syntax.c, but one shows up in search.c.

There is also a warning from movemail.c:

    warning: the use of `mktemp' is dangerous, better use `mkstemp' or
    `mkdtemp'

Are these significant?  Should I be worried?

Shortened link to launchpad.net build log: http://xrl.us/bbntv

Mark.

-- 
http://hexmode.com/
GPG Fingerprint: 7E15 362D A32C DFAB E4D2  B37A 735E F10A 2DFC BFF5

The most beautiful experience we can have is the mysterious.
    -- Albert Einstein, The World As I See it

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23  4:57 unicode build: "comparison is always true" warnings Mark A. Hershberger
@ 2007-11-23 15:09 ` Stefan Monnier
  2007-11-23 17:42   ` Mark A. Hershberger
  0 siblings, 1 reply; 26+ messages in thread
From: Stefan Monnier @ 2007-11-23 15:09 UTC (permalink / raw)
  To: Mark A. Hershberger; +Cc: emacs-devel

> I'm working on providing snapshots of the unicode branch and have been
> getting a ton of these messages:

>     warning: comparison is always true due to limited range of data type

> Most are in syntax.c, but one shows up in search.c.

These are difficult to eliminate without making the code uglier.
IIRC the issue is typically that there's are generic macros to handle
characters and they can be used on any character (not just on 8-bit
chars).  But when you use it on a "char", part of the macro becomes
trivially optimizable and gcc thinks this may in fact indicate the
presence of a bug.
Maybe we should just silence these warnings with the
appropriate -Wno-<foobar>.

> There is also a warning from movemail.c:

>     warning: the use of `mktemp' is dangerous, better use `mkstemp' or
>     `mkdtemp'

This one looks bad.


        Stefan

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 15:09 ` Stefan Monnier
@ 2007-11-23 17:42   ` Mark A. Hershberger
  2007-11-23 18:07     ` Andreas Schwab
  0 siblings, 1 reply; 26+ messages in thread
From: Mark A. Hershberger @ 2007-11-23 17:42 UTC (permalink / raw)
  To: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> There is also a warning from movemail.c:
>>     warning: the use of `mktemp' is dangerous, better use `mkstemp' or
>>     `mkdtemp'
>
> This one looks bad.

I attempted to fix this.  It has been ages since I had to write C, but
this fix is almost identical to the one here: http://xrl.us/bbocw

I'll apply if this looks ok.

diff -u -b -r1.88 movemail.c
--- movemail.c	26 Jul 2007 05:26:12 -0000	1.88
+++ movemail.c	23 Nov 2007 17:38:40 -0000
@@ -325,14 +325,20 @@
 	p--;
       *p = 0;
       strcpy (p, "EXXXXXX");
+#ifndef HAVE_MKSTEMP
       mktemp (tempname);
       unlink (tempname);
+#endif
 
       while (1)
 	{
 	  /* Create the lock file, but not under the lock file name.  */
 	  /* Give up if cannot do that.  */
+#ifndef HAVE_MKSTEMP
 	  desc = open (tempname, O_WRONLY | O_CREAT | O_EXCL, 0666);
+#else
+          desc = mkstemp (tempname);
+#endif
 	  if (desc < 0)
 	    {
 	      char *message = (char *) xmalloc (strlen (tempname) + 50);




-- 
http://hexmode.com/
GPG Fingerprint: 7E15 362D A32C DFAB E4D2  B37A 735E F10A 2DFC BFF5

The most beautiful experience we can have is the mysterious.
    -- Albert Einstein, The World As I See it

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 17:42   ` Mark A. Hershberger
@ 2007-11-23 18:07     ` Andreas Schwab
  2007-11-23 18:40       ` Stefan Monnier
  2007-11-23 18:42       ` Mark A. Hershberger
  0 siblings, 2 replies; 26+ messages in thread
From: Andreas Schwab @ 2007-11-23 18:07 UTC (permalink / raw)
  To: Mark A. Hershberger; +Cc: emacs-devel

mah@everybody.org (Mark A. Hershberger) writes:

>        while (1)
>  	{
>  	  /* Create the lock file, but not under the lock file name.  */
>  	  /* Give up if cannot do that.  */
> +#ifndef HAVE_MKSTEMP
>  	  desc = open (tempname, O_WRONLY | O_CREAT | O_EXCL, 0666);
> +#else
> +          desc = mkstemp (tempname);
> +#endif

That won't work since mkstemp modifies tempname in place, but can be
called more than once here (the second call will return with EINVAL).

(The use of mktemp is not really bad here anyway.)

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 18:07     ` Andreas Schwab
@ 2007-11-23 18:40       ` Stefan Monnier
  2007-11-23 18:52         ` Andreas Schwab
  2007-11-23 18:42       ` Mark A. Hershberger
  1 sibling, 1 reply; 26+ messages in thread
From: Stefan Monnier @ 2007-11-23 18:40 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Mark A. Hershberger, emacs-devel

> (The use of mktemp is not really bad here anyway.)

Why not?  (don't hesitate to put the explanation in the source code, of
course).


        Stefan

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 18:07     ` Andreas Schwab
  2007-11-23 18:40       ` Stefan Monnier
@ 2007-11-23 18:42       ` Mark A. Hershberger
  2007-11-23 18:54         ` Andreas Schwab
  1 sibling, 1 reply; 26+ messages in thread
From: Mark A. Hershberger @ 2007-11-23 18:42 UTC (permalink / raw)
  To: emacs-devel

Andreas Schwab <schwab@suse.de> writes:

> mah@everybody.org (Mark A. Hershberger) writes:
>
>>        while (1)
>>  	{
>>  	  /* Create the lock file, but not under the lock file name.  */
>>  	  /* Give up if cannot do that.  */
>> +#ifndef HAVE_MKSTEMP
>>  	  desc = open (tempname, O_WRONLY | O_CREAT | O_EXCL, 0666);
>> +#else
>> +          desc = mkstemp (tempname);
>> +#endif
>
> That won't work since mkstemp modifies tempname in place, but can be
> called more than once here (the second call will return with EINVAL).

There is only one call.

If you look at the full patch, mktemp (not mkstemp) is called once if
HAVE_MKSTEMP is _not_ defined.  Here, mkstemp (not mktemp) is called if
HAVE_MKSTEMP _is_ defined.

Mark.

-- 
http://hexmode.com/
GPG Fingerprint: 7E15 362D A32C DFAB E4D2  B37A 735E F10A 2DFC BFF5

The most beautiful experience we can have is the mysterious.
    -- Albert Einstein, The World As I See it

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 18:40       ` Stefan Monnier
@ 2007-11-23 18:52         ` Andreas Schwab
  2007-11-23 20:02           ` Stefan Monnier
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Schwab @ 2007-11-23 18:52 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Mark A. Hershberger, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> (The use of mktemp is not really bad here anyway.)
>
> Why not?

Because it is used securely.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 18:42       ` Mark A. Hershberger
@ 2007-11-23 18:54         ` Andreas Schwab
  0 siblings, 0 replies; 26+ messages in thread
From: Andreas Schwab @ 2007-11-23 18:54 UTC (permalink / raw)
  To: Mark A. Hershberger; +Cc: emacs-devel

mah@everybody.org (Mark A. Hershberger) writes:

> Andreas Schwab <schwab@suse.de> writes:
>
>> mah@everybody.org (Mark A. Hershberger) writes:
>>
>>>        while (1)
>>>  	{
>>>  	  /* Create the lock file, but not under the lock file name.  */
>>>  	  /* Give up if cannot do that.  */
>>> +#ifndef HAVE_MKSTEMP
>>>  	  desc = open (tempname, O_WRONLY | O_CREAT | O_EXCL, 0666);
>>> +#else
>>> +          desc = mkstemp (tempname);
>>> +#endif
>>
>> That won't work since mkstemp modifies tempname in place, but can be
>> called more than once here (the second call will return with EINVAL).
>
> There is only one call.

Inside a loop.

> If you look at the full patch, mktemp (not mkstemp) is called once if
> HAVE_MKSTEMP is _not_ defined.  Here, mkstemp (not mktemp) is called if
> HAVE_MKSTEMP _is_ defined.

Correct but irrelevant.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 18:52         ` Andreas Schwab
@ 2007-11-23 20:02           ` Stefan Monnier
  2007-11-23 20:12             ` Glenn Morris
  2007-11-23 21:22             ` Andreas Schwab
  0 siblings, 2 replies; 26+ messages in thread
From: Stefan Monnier @ 2007-11-23 20:02 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Mark A. Hershberger, emacs-devel

>>> (The use of mktemp is not really bad here anyway.)
>> Why not?
> Because it is used securely.

I assumed that's what you meant.  The question is: why is it secure in
this case?


        Stefan

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 20:02           ` Stefan Monnier
@ 2007-11-23 20:12             ` Glenn Morris
  2007-11-23 21:22             ` Andreas Schwab
  1 sibling, 0 replies; 26+ messages in thread
From: Glenn Morris @ 2007-11-23 20:12 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Andreas Schwab, Mark A. Hershberger, emacs-devel

Stefan Monnier wrote:

>> Because it is used securely.
>
> I assumed that's what you meant.  The question is: why is it secure in
> this case?

I think Andreas must pay for email by the word... ;)

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 20:02           ` Stefan Monnier
  2007-11-23 20:12             ` Glenn Morris
@ 2007-11-23 21:22             ` Andreas Schwab
  2007-11-23 21:35               ` Mark A. Hershberger
  1 sibling, 1 reply; 26+ messages in thread
From: Andreas Schwab @ 2007-11-23 21:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Mark A. Hershberger, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>>> (The use of mktemp is not really bad here anyway.)
>>> Why not?
>> Because it is used securely.
>
> I assumed that's what you meant.  The question is: why is it secure in
> this case?

Please tell me which part of the sources you have difficulties to
understand, then I can explain it to you.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 21:22             ` Andreas Schwab
@ 2007-11-23 21:35               ` Mark A. Hershberger
  2007-11-23 21:53                 ` Andreas Schwab
  0 siblings, 1 reply; 26+ messages in thread
From: Mark A. Hershberger @ 2007-11-23 21:35 UTC (permalink / raw)
  To: emacs-devel

Andreas Schwab <schwab@suse.de> writes:

> Please tell me which part of the sources you have difficulties to
> understand, then I can explain it to you.

Other people will see the compilation warning and naively ask “Why is
emacs doing things insecurely?”

It would be helpful to have an explanation in the comments to explain
_why_ it is secure despite the warnings.

I'll readily admit I don't know enough about mailfile locking or mktemp
to understand why this is secure despite the warnings.  For those of us
who don't readily understand secure mailfile locking, could you explain
why mktemp is ok?

-- 
http://hexmode.com/
GPG Fingerprint: 7E15 362D A32C DFAB E4D2  B37A 735E F10A 2DFC BFF5

The most beautiful experience we can have is the mysterious.
    -- Albert Einstein, The World As I See it

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 21:35               ` Mark A. Hershberger
@ 2007-11-23 21:53                 ` Andreas Schwab
  2007-11-24  1:49                   ` Mark A. Hershberger
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Schwab @ 2007-11-23 21:53 UTC (permalink / raw)
  To: Mark A. Hershberger; +Cc: emacs-devel

mah@everybody.org (Mark A. Hershberger) writes:

> I'll readily admit I don't know enough about mailfile locking or mktemp
> to understand why this is secure despite the warnings.  For those of us
> who don't readily understand secure mailfile locking, could you explain
> why mktemp is ok?

Please tell me what you think is unsecure here?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-23 21:53                 ` Andreas Schwab
@ 2007-11-24  1:49                   ` Mark A. Hershberger
  2007-11-24  8:26                     ` Andreas Schwab
  0 siblings, 1 reply; 26+ messages in thread
From: Mark A. Hershberger @ 2007-11-24  1:49 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

Andreas Schwab <schwab@suse.de> writes:

> Please tell me what you think is unsecure here?

mktemp

-- 
http://hexmode.com/
GPG Fingerprint: 7E15 362D A32C DFAB E4D2  B37A 735E F10A 2DFC BFF5

The most beautiful experience we can have is the mysterious.
    -- Albert Einstein, The World As I See it

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24  1:49                   ` Mark A. Hershberger
@ 2007-11-24  8:26                     ` Andreas Schwab
  2007-11-24  9:30                       ` Jan Djärv
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Schwab @ 2007-11-24  8:26 UTC (permalink / raw)
  To: Mark A. Hershberger; +Cc: emacs-devel

mah@everybody.org (Mark A. Hershberger) writes:

> Andreas Schwab <schwab@suse.de> writes:
>
>> Please tell me what you think is unsecure here?
>
> mktemp

Please expand.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24  8:26                     ` Andreas Schwab
@ 2007-11-24  9:30                       ` Jan Djärv
  2007-11-24  9:46                         ` Andreas Schwab
  0 siblings, 1 reply; 26+ messages in thread
From: Jan Djärv @ 2007-11-24  9:30 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Mark A. Hershberger, emacs-devel

 From the man page of mktemp:

        Since on the one  hand  the  names
        are  easy to guess, and on the other hand there is a race between test-
        ing whether the name exists and opening the file, every use of mktemp()
        is a security risk.  The race is avoided by mkstemp(3).



Andreas Schwab skrev:
> mah@everybody.org (Mark A. Hershberger) writes:
> 
>> Andreas Schwab <schwab@suse.de> writes:
>>
>>> Please tell me what you think is unsecure here?
>> mktemp
> 
> Please expand.
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24  9:30                       ` Jan Djärv
@ 2007-11-24  9:46                         ` Andreas Schwab
  2007-11-24 11:23                           ` Matthieu Lemerre
       [not found]                           ` <4747FDF6.3050203@swipnet.se>
  0 siblings, 2 replies; 26+ messages in thread
From: Andreas Schwab @ 2007-11-24  9:46 UTC (permalink / raw)
  To: Jan Djärv; +Cc: Mark A. Hershberger, emacs-devel

Jan Djärv <jan.h.d@swipnet.se> writes:

> From the man page of mktemp:
>
>        Since on the one  hand  the  names
>        are  easy to guess, and on the other hand there is a race between test-
>        ing whether the name exists and opening the file, every use of mktemp()
>        is a security risk.  The race is avoided by mkstemp(3).

In which way it is different from what mkstemp is doing?

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24  9:46                         ` Andreas Schwab
@ 2007-11-24 11:23                           ` Matthieu Lemerre
  2007-11-24 14:16                             ` Andreas Schwab
       [not found]                           ` <4747FDF6.3050203@swipnet.se>
  1 sibling, 1 reply; 26+ messages in thread
From: Matthieu Lemerre @ 2007-11-24 11:23 UTC (permalink / raw)
  To: emacs-devel


Andreas Schwab <schwab@suse.de> writes:
>
> In which way it is different from what mkstemp is doing?
>

When you use mktemp, you have to use two operations:
1/Create a file name with mktemp
2/Open it

An attacker could create a symlink with this name file between these
two operations, thus the file would be erased.

mkstemp guarantees these two operations to be done atomically by the
kernel (and the kernel chooses a name corresponding to an unused file
name), so the race condition does not work in that case; the file
descriptor is guaranteed to always correspond to a new file.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
       [not found]                                 ` <jelk8nrido.fsf@sykes.suse.de>
@ 2007-11-24 13:08                                   ` Jan Djärv
  2007-11-24 13:57                                     ` Andreas Schwab
  0 siblings, 1 reply; 26+ messages in thread
From: Jan Djärv @ 2007-11-24 13:08 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Emacs-Devel

Andreas Schwab skrev:
> Jan Djärv <jan.h.d@swipnet.se> writes:
> 
>> Ok.  But mkstemp has one advantage that mktemp doesn't have, it does not emit
>> a warning when compiling :-).  I think that is cause enough to use mkstemp
>> where available, even if they both are equivalent.
> 
> It also adds complexity to a already quite complex program for its
> task.  And as the first attempt shows, it is not trivial to use mkstemp
> without breaking the program.
> 

The patch just needs some small changes:


*** movemail.c.~1.88.~  2007-07-27 09:52:07.000000000 +0200
--- movemail.c  2007-11-24 14:07:13.000000000 +0100
***************
*** 324,338 ****
--- 324,345 ----
        while (p != tempname && !IS_DIRECTORY_SEP (p[-1]))
        p--;
        *p = 0;
+ #ifndef HAVE_MKSTEMP
        strcpy (p, "EXXXXXX");
        mktemp (tempname);
        unlink (tempname);
+ #endif

        while (1)
        {
          /* Create the lock file, but not under the lock file name.  */
          /* Give up if cannot do that.  */
+ #ifndef HAVE_MKSTEMP
          desc = open (tempname, O_WRONLY | O_CREAT | O_EXCL, 0666);
+ #else
+           strcpy (p, "EXXXXXX");
+           desc = mkstemp (tempname);
+ #endif
          if (desc < 0)
            {
              char *message = (char *) xmalloc (strlen (tempname) + 50);

	Jan D.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24 13:08                                   ` Jan Djärv
@ 2007-11-24 13:57                                     ` Andreas Schwab
  2007-11-24 18:24                                       ` Mark A. Hershberger
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Schwab @ 2007-11-24 13:57 UTC (permalink / raw)
  To: Jan Djärv; +Cc: Emacs-Devel

Jan Djärv <jan.h.d@swipnet.se> writes:

> The patch just needs some small changes:
>
>
> *** movemail.c.~1.88.~  2007-07-27 09:52:07.000000000 +0200
> --- movemail.c  2007-11-24 14:07:13.000000000 +0100
> ***************
> *** 324,338 ****
> --- 324,345 ----
>         while (p != tempname && !IS_DIRECTORY_SEP (p[-1]))
>         p--;
>         *p = 0;
> + #ifndef HAVE_MKSTEMP
>         strcpy (p, "EXXXXXX");
>         mktemp (tempname);
>         unlink (tempname);
> + #endif
>
>         while (1)
>         {
>           /* Create the lock file, but not under the lock file name.  */
>           /* Give up if cannot do that.  */
> + #ifndef HAVE_MKSTEMP
>           desc = open (tempname, O_WRONLY | O_CREAT | O_EXCL, 0666);
> + #else
> +           strcpy (p, "EXXXXXX");
> +           desc = mkstemp (tempname);
> + #endif
>           if (desc < 0)
>             {
>               char *message = (char *) xmalloc (strlen (tempname) + 50);

I'd suggest moving the mktemp into the loop as well.  This will avoid
code duplication.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24 11:23                           ` Matthieu Lemerre
@ 2007-11-24 14:16                             ` Andreas Schwab
  2007-11-24 14:49                               ` Matthieu Lemerre
  2007-11-24 15:20                               ` David Kastrup
  0 siblings, 2 replies; 26+ messages in thread
From: Andreas Schwab @ 2007-11-24 14:16 UTC (permalink / raw)
  To: Matthieu Lemerre; +Cc: emacs-devel

Matthieu Lemerre <racin@free.fr> writes:

> Andreas Schwab <schwab@suse.de> writes:
>>
>> In which way it is different from what mkstemp is doing?
>>
>
> When you use mktemp, you have to use two operations:
> 1/Create a file name with mktemp
> 2/Open it

That's the same what mkstemp does.

> An attacker could create a symlink with this name file between these
> two operations, thus the file would be erased.

That does not happen, because the open will fail when the file already
exists.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24 14:16                             ` Andreas Schwab
@ 2007-11-24 14:49                               ` Matthieu Lemerre
  2007-11-24 15:20                               ` David Kastrup
  1 sibling, 0 replies; 26+ messages in thread
From: Matthieu Lemerre @ 2007-11-24 14:49 UTC (permalink / raw)
  To: emacs-devel


>> An attacker could create a symlink with this name file between these
>> two operations, thus the file would be erased.
>
> That does not happen, because the open will fail when the file already
> exists.
>
> Andreas.

You're right, this is the meaning of O_EXCL, that I had forgotten.

Then I agree with you; mkstemp is just a commodity.

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24 14:16                             ` Andreas Schwab
  2007-11-24 14:49                               ` Matthieu Lemerre
@ 2007-11-24 15:20                               ` David Kastrup
  2007-11-24 17:00                                 ` Andreas Schwab
  1 sibling, 1 reply; 26+ messages in thread
From: David Kastrup @ 2007-11-24 15:20 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

Andreas Schwab <schwab@suse.de> writes:

> Matthieu Lemerre <racin@free.fr> writes:
>
>> An attacker could create a symlink with this name file between these
>> two operations, thus the file would be erased.
>
> That does not happen, because the open will fail when the file already
> exists.

And I bet it was impossible for you to mention this in the last 6 mails
or so.  After all, the system call could possibly have been considered
unsafe because of a buffer overflow or something (and that's so
absolutely very likely what GCC tries warning about).  So before
accidently giving one pointer too many, better taunt the other to go out
on a limb before showing him wrong.

There is probably something educational hidden in there, but I am likely
too dull to discern it.

Sorry for the rant.  I just think that at times we could be more helpful
towards each other.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24 15:20                               ` David Kastrup
@ 2007-11-24 17:00                                 ` Andreas Schwab
  2007-11-24 18:21                                   ` David Kastrup
  0 siblings, 1 reply; 26+ messages in thread
From: Andreas Schwab @ 2007-11-24 17:00 UTC (permalink / raw)
  To: David Kastrup; +Cc: emacs-devel

David Kastrup <dak@gnu.org> writes:

> Andreas Schwab <schwab@suse.de> writes:
>
>> Matthieu Lemerre <racin@free.fr> writes:
>>
>>> An attacker could create a symlink with this name file between these
>>> two operations, thus the file would be erased.
>>
>> That does not happen, because the open will fail when the file already
>> exists.
>
> And I bet it was impossible for you to mention this in the last 6 mails
> or so.

I have asked several times what it is considered insecure, but that was
the first mail that gave a hint.  I cannot read anyone's mind.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24 17:00                                 ` Andreas Schwab
@ 2007-11-24 18:21                                   ` David Kastrup
  0 siblings, 0 replies; 26+ messages in thread
From: David Kastrup @ 2007-11-24 18:21 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: emacs-devel

Andreas Schwab <schwab@suse.de> writes:

> David Kastrup <dak@gnu.org> writes:
>
>> Andreas Schwab <schwab@suse.de> writes:
>>
>>> Matthieu Lemerre <racin@free.fr> writes:
>>>
>>>> An attacker could create a symlink with this name file between these
>>>> two operations, thus the file would be erased.
>>>
>>> That does not happen, because the open will fail when the file already
>>> exists.
>>
>> And I bet it was impossible for you to mention this in the last 6 mails
>> or so.
>
> I have asked several times what it is considered insecure, but that
> was the first mail that gave a hint.  I cannot read anyone's mind.

What a delicately selective quotation for the final cheap shot.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: unicode build: "comparison is always true" warnings
  2007-11-24 13:57                                     ` Andreas Schwab
@ 2007-11-24 18:24                                       ` Mark A. Hershberger
  0 siblings, 0 replies; 26+ messages in thread
From: Mark A. Hershberger @ 2007-11-24 18:24 UTC (permalink / raw)
  To: emacs-devel

Andreas Schwab <schwab@suse.de> writes:

> I'd suggest moving the mktemp into the loop as well.  This will avoid
> code duplication.

like this?

--- movemail.c	26 Jul 2007 05:26:12 -0000	1.88
+++ movemail.c	24 Nov 2007 18:23:43 -0000
@@ -324,15 +324,20 @@
       while (p != tempname && !IS_DIRECTORY_SEP (p[-1]))
 	p--;
       *p = 0;
-      strcpy (p, "EXXXXXX");
-      mktemp (tempname);
-      unlink (tempname);
 
       while (1)
 	{
 	  /* Create the lock file, but not under the lock file name.  */
 	  /* Give up if cannot do that.  */
+          strcpy (p, "EXXXXXX");
+#ifndef HAVE_MKSTEMP
+          mktemp (tempname);
+          unlink (tempname);
 	  desc = open (tempname, O_WRONLY | O_CREAT | O_EXCL, 0666);
+#else
+          desc = mkstemp (tempname);
+#endif
+
 	  if (desc < 0)
 	    {
 	      char *message = (char *) xmalloc (strlen (tempname) + 50);

-- 
http://hexmode.com/
GPG Fingerprint: 7E15 362D A32C DFAB E4D2  B37A 735E F10A 2DFC BFF5

The most beautiful experience we can have is the mysterious.
    -- Albert Einstein, The World As I See it

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2007-11-24 18:24 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-11-23  4:57 unicode build: "comparison is always true" warnings Mark A. Hershberger
2007-11-23 15:09 ` Stefan Monnier
2007-11-23 17:42   ` Mark A. Hershberger
2007-11-23 18:07     ` Andreas Schwab
2007-11-23 18:40       ` Stefan Monnier
2007-11-23 18:52         ` Andreas Schwab
2007-11-23 20:02           ` Stefan Monnier
2007-11-23 20:12             ` Glenn Morris
2007-11-23 21:22             ` Andreas Schwab
2007-11-23 21:35               ` Mark A. Hershberger
2007-11-23 21:53                 ` Andreas Schwab
2007-11-24  1:49                   ` Mark A. Hershberger
2007-11-24  8:26                     ` Andreas Schwab
2007-11-24  9:30                       ` Jan Djärv
2007-11-24  9:46                         ` Andreas Schwab
2007-11-24 11:23                           ` Matthieu Lemerre
2007-11-24 14:16                             ` Andreas Schwab
2007-11-24 14:49                               ` Matthieu Lemerre
2007-11-24 15:20                               ` David Kastrup
2007-11-24 17:00                                 ` Andreas Schwab
2007-11-24 18:21                                   ` David Kastrup
     [not found]                           ` <4747FDF6.3050203@swipnet.se>
     [not found]                             ` <je1wagrkn9.fsf@sykes.suse.de>
     [not found]                               ` <4748064D.6010405@swipnet.se>
     [not found]                                 ` <jelk8nrido.fsf@sykes.suse.de>
2007-11-24 13:08                                   ` Jan Djärv
2007-11-24 13:57                                     ` Andreas Schwab
2007-11-24 18:24                                       ` Mark A. Hershberger
2007-11-23 18:42       ` Mark A. Hershberger
2007-11-23 18:54         ` Andreas Schwab

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.