Sign in to follow this  
Storyyeller

Strange c code with var args type mismatch

Recommended Posts

Storyyeller    215
I've been looking through the Qemu-KVM sources and I am at a loss to explain how this coe could possibly work.

As I understand var_args, it is critical that the correct types are used. If you call va_arg with the wrong type, it will just blindly pop that many bytes off the stack, and probably corrupt the whole stack. However, the function is being called with an int as the third parameter, while a void* is being expected, and these are different sizes on a 64bit platform. Yet I've built it in 64bit and it runs perfectly fine. How could this be?


[source lang="cpp"]int kvm_vm_ioctl(KVMState *s, int type, ...)
{
int ret;
void *arg;
va_list ap;
va_start(ap, type);
arg = va_arg(ap, void *);
va_end(ap);
ret = ioctl(s->vmfd, type, arg);
if (ret == -1)
ret = -errno;
return ret;
}

int kvm_init_vcpu(CPUState *env)
{
KVMState *s = kvm_state;
long mmap_size;
int ret;
dprintf("kvm_init_vcpu\n");
ret = kvm_vm_ioctl(s, KVM_CREATE_VCPU, env->cpu_index);
if (ret < 0)
{
dprintf("kvm_create_vcpu failed\n");
goto err;
}
env->kvm_fd = ret;
env->kvm_state = s;
mmap_size = kvm_ioctl(s, KVM_GET_VCPU_MMAP_SIZE, 0);
if (mmap_size < 0)
{
dprintf("KVM_GET_VCPU_MMAP_SIZE failed\n");
goto err;
}
env->kvm_run = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED,
env->kvm_fd, 0);
if (env->kvm_run == MAP_FAILED) {
ret = -errno;
dprintf("mmap'ing vcpu state failed\n");
goto err;
}
ret = kvm_arch_init_vcpu(env);
if (ret == 0) {
qemu_register_reset(kvm_reset_vcpu, env);
kvm_arch_reset_vcpu(env);
ret = kvm_arch_put_registers(env);
}
err:
return ret;
}
[/source] Edited by ApochPiQ
Formatted code to make it readable

Share this post


Link to post
Share on other sites
ApochPiQ    23003
This is basically a fluke. In x64 calling conventions, parameters passed via varargs must be present in stack slots. A stack slot is defined by convention to be 64 bits wide. Therefore, the code will place 32 bits into a 64 bit slot on the stack, and the callee will pop a 64 bit slot off the stack. The upper 32 bits will be undefined; if the code recasts to an int later, and discards those bits, everything is fine (but only by sheer luck). If those upper 32 bits are ever examined, they will be bogus.

You should be able to confirm this by looking at the disassembly of the program.

Share this post


Link to post
Share on other sites
Storyyeller    215
Well arg is immediately passed to ioctl. So I guess I'd have to look through the kernel module code to see how it's being used?

Also, how do you get code to show up properly?

Share this post


Link to post
Share on other sites
ApochPiQ    23003
Turns out the problem is in the way you used the source tag. If you do [ source = "language" ] the forum software mangles the post; you have to do the "correct" tag syntax which is [ source lang="language" ] to get it to cooperate.

Lame, I know.

Share this post


Link to post
Share on other sites
Storyyeller    215
Well I looked into the module. Apparently the signature for Ioctl takes an unsigned long as the third argument, but it then calls a function with a u32. So it only uses the lower 32 bits after all.


The complete list of casts for this particular function is
int -> void* -> unsigned long -> u32

Why do they have to make these things so complicated?

Share this post


Link to post
Share on other sites
frob    44908
[quote name='Storyyeller' timestamp='1311570112' post='4839855']
Well I looked into the module. Apparently the signature for Ioctl takes an unsigned long as the third argument, but it then calls a function with a u32. So it only uses the lower 32 bits after all.


The complete list of casts for this particular function is
int -> void* -> unsigned long -> u32

Why do they have to make these things so complicated?
[/quote]

Things like this --- kernel development like Kernel Virtual Machines project under discussion --- are designed and written and optimized for the machine running it, not for the convenience of the humans working on the code.

Generally at this level they are not assumptions about the machine; instead they are very intentionally chosen design decisions made to shave a few nanoseconds from the execution time. The extra hours for the human experts who maintain the system are more than made up for in the higher performance for the millions of machines that are executing it, around the clock, around the world.


They are trying to make it best for the end user. The end user of these systems is not the programmers, it is the high-performance applications.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this