1

I'm storing a series of pids (i.e. Linux process ids) in an array of longs. I realize that a pid is not a long, but I have no choice in terms of using a different variable type.

The issue I'm having occurs when I try and print the pid using printf. If I print the long that stores the pid using %ld, I get the wrong pids:

8435315771308 process_ancesto
8358006359962 bash
8353711392665 login
4294967297 init
0 swapper/0

However, if I print using %d (which generates a compiler warning), I get the correct result (i.e. the result returned by typing ps into the terminal):

1969 process_ancesto
1946 bash
1945 login
1 init
0 swapper/0

What is causing this behaviour? A pid -> long cast is a widening conversion and shouldn't cause any problems.

Here is the program I use to make the system call that returns the pids:

int main(int argc, char *argv[])
{
    struct process_info arr[10];
    long n=0;
    int result = syscall(_CS300_PROCESS_ANCESTORS_, arr,10,&n);
    assert(result==0);
    for(int i=0;i<n;i++) {
        printf("%d %s\n",arr[i].pid,arr[i].name);
    }
    return 0;
}

If I replace the %d with %ld it prints incorrect info.

Here is the line from my system call where I record the pid:

if(copy_long_to_user(&info_array[num_filled_kernel].pid, &curr_task->pid)) {                
            return -EFAULT;
        }

info_array[num_filled_kernel].pid is a long.

Adam
  • 8,752
  • 12
  • 54
  • 96

4 Answers4

6

Two problems:

In your copy_to_user, the second argument is a pointer to pid_t, it appears, which as you say is an int (32 bits). So you are copying 64 bits from a 32-bit variable; the remaining 32 bits (the high half, since x86 is little-endian) will be filled with whatever came next in memory, if you are lucky. (If you are unlucky you get a fault here.) Integer conversions aren't done when you access things via pointers.

The simplest way to do this safely is to use a temporary variable:

long tmp = curr_task->pid; // sign-extension done here
copy_long_to_user(..., &tmp);

Then in your user space code, you are using the %d format specifier to print something that's apparently a long. This won't work; printf, being a variadic function, doesn't know the type that its arguments are expected to have, and so can't convert them appropriately. If you're going to pass a long, use the %ld format specifier.

Nate Eldredge
  • 48,811
  • 6
  • 54
  • 82
  • [I agree with the solution](http://stackoverflow.com/a/36658968/103167), but that isn't a "temporary". – Ben Voigt Apr 16 '16 at 00:50
  • 1
    @BenVoigt: To be grammatical I should have said "temporary variable". I meant it in the informal sense of the word, ["a variable whose purpose is short-lived"](https://en.wikipedia.org/wiki/Temporary_variable), not in any formal language-standard sense. – Nate Eldredge Apr 16 '16 at 00:53
  • @BenVoigt: Oh, I see that was a previous question by the same user. Awkward that they overlapped this way. – Nate Eldredge Apr 16 '16 at 01:01
  • Would a cast also work? I.e. `copy_long_to_user(... , &((long)curr_task->pid))` – Adam Apr 16 '16 at 01:02
  • 1
    @Adam: As Ben Voigt explained in his answer to your other question, no it will not. [The result of a cast isn't an lvalue](http://stackoverflow.com/a/21389579/634919) and you can't take its address. (Some old versions of gcc supported something like this as an extension, but it's non-standard and can't be expected to work these days.) – Nate Eldredge Apr 16 '16 at 01:03
  • Why do programmers so often reach for a cast when they're not even sure exactly what it does? – Michael Burr Apr 16 '16 at 01:08
  • Why don't I need to cast `curr_task->pid` when it's assigned to tmp? – Adam Apr 16 '16 at 01:12
  • 1
    Because the compiler can perform an implicit conversion from `int` to `long`. – Michael Burr Apr 16 '16 at 01:18
2

A pid -> long cast is a widening conversion and shouldn't cause any problems.

Sure, but unless you're actually performing said cast it won't happen, at least not with varargs/stdargs. Either cast the argument yourself or use the correct specifier.

Ignacio Vazquez-Abrams
  • 776,304
  • 153
  • 1,341
  • 1,358
  • What happens if I cast something and then use the address operator on it? That's what I think I need to do with `copy_to_user`. How can one address refer to two different variable types? – Adam Apr 16 '16 at 00:45
  • 1
    You need to think of casts and address/dereference operators as no-ops; they change the type for the C compiler, but in the vast majority of cases they don't affect the data in any way. – Ignacio Vazquez-Abrams Apr 16 '16 at 00:47
  • So if I cast a `pid` to a long, the underlying "pid-ness" of the `pid` variable doesn't change. The cast just allows us to fit a copy of the pid variable into a long-type variable. – Adam Apr 16 '16 at 00:49
  • It tells the compiler that it should treat it as a 64-bit value instead of a 32-bit value. If something other than 0 is in the other 32 bits then you get results similar to yours. – Ignacio Vazquez-Abrams Apr 16 '16 at 00:51
2

If you take a look at your numbers as hex numbers

8358006359962 bash

is in hex

79A0000079A

and 

1946 bash

is in hex

79A

So guessing here -- but you have done something bad when you converted the numbers into long longs in copy_long_to_user, since the 79A sequence repeats in the higher bits.

Soren
  • 14,402
  • 4
  • 41
  • 67
2

So the pid is copied into the output array using:

copy_long_to_user(&info_array[num_filled_kernel].pid, &curr_task->pid)

If curr_task is a struct task_struct*, then curr_task->pid is an int, which is 4 bytes on Linux (whether 32 or 64 bits). However long on 64-bit Linux is 8 bytes, so you end up copying not only the pid, but 4 other adjacent bytes (which are probably the tgid - I don't know exactly what that is, but it looks like it often has the same value as the pid).

Update: the tgid is a Thread Group ID, and it does often have the same value as the pid:

Community
  • 1
  • 1
Michael Burr
  • 333,147
  • 50
  • 533
  • 760