Page MenuHome GnuPG

Fix FD2INT for 64-bit Windows
Testing, NormalPublic

Description

FD2INT implementation as of today (2023-07-18) has potential issue.

  • It was written for 32-bit Windows and POSIX
  • For 32-bit Windows, casting with (unsigned int) works (it means, no conversion of the value)
  • For 64-bit Windows, casting with (unsigned int) means conversion of lower 32-bit.
    • This works mostly, for the case of 32-bit value of HANDLE
    • When HANDLE is within the range of 32-bit, it's OK
    • But, it may fail to represent INVALID FD (0xffffffffffffffff), the converted value is 0xffffffff

Related Objects

Event Timeline

gniibe triaged this task as Normal priority.Jul 18 2023, 7:15 AM
gniibe created this task.
gniibe created this object with edit policy "Contributor (Project)".

Here is a test program for 64-bit Windows to see how cast works:

#include <stdint.h>
#include <stdio.h>

int
main (int argc, const char *argv[])
{
  const char *p;
  uint64_t n0, n1;
  int n;

  p = (void *)-1;
  /* warning: cast from pointer to integer of different size [-Wpointer-to-int-cast] */
  n0 = (unsigned int)p;         /* Not good.  It's masked.  */
  printf ("0: %p : %016llx\n", p, n0);
  n1 = (uintptr_t)p;            /* Good.  It's kept as is.  */
  printf ("1: %p : %016llx\n", p, n1);

  n = -1;

  /* works by sign extend, but warning for different size. */
  /* warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] */
  p = (void *)n;
  printf ("ok:   %p\n", p);

  /* works by sign extend, explicitly request sign extend. */
  p = (void *)(intptr_t)n;
  printf ("good: %p\n", p);

  /* works by no sign extend.  */
  p = (void *)(uintptr_t)n;
  printf ("ok:   %p\n", p);

  /* doesn't works by no sign extend.  */
  p = (void *)(uintptr_t)(unsigned int)n;
  printf ("bad:  %p\n", p);

  return 0;
}

The result is:

$ ./a.exe 
0: ffffffffffffffff : 00000000ffffffff
1: ffffffffffffffff : ffffffffffffffff
ok:   ffffffffffffffff
good: ffffffffffffffff
ok:   ffffffffffffffff
bad:  00000000ffffffff

We need to fix the macro FD2INT for 64-bit Windows.

Fixed FD2INT in: rG521ec40aea89: common,w32: Fix FD2INT macro.

Use of FD2INT for the first argument of select is semantically not good. It's the number of file descriptor. When we use FD2INT here, the type is converted to 64-bit integer, then implicitly demoted to 32-bit integer. We need new macro, say, FD2NUM to convert FD into 32-bit integer.
<--- done in: rGea1935252e28: commond: Introduce FD2NUM to express conversion to number of fds.

gniibe changed the task status from Open to Testing.Jul 19 2023, 4:53 AM

On 64-bit Windows, the situation now is:

  • FD2INT
    • Fixed for 64-bit. No masking to lower-bit any more.
  • FD2NUM
    • It's conversion to 32-bit signed integer. It is used for select call. In the select implementation, the value is not used. So, any conversion works.
    • When/if POSIX compliant select will be introduced in future, we will be able to fix the FD2NUM macro, together with the select calls.
  • FD_DBG
    • It's conversion to 32-bit signed integer. It is used to display debug message using FD. Converted value may not be accurate.
    • When/if OS will be changed in future and we will want to display accurately, we will be able to fix this FD_DBG macro, together with formatting string.

That is, everything works well now. All places with the cast have been examined and fixed when there were problems.

gniibe mentioned this in Unknown Object (Event).Jul 24 2023, 8:30 AM