* Peter Stuge peter@stuge.se [070712 00:49]:
+static int lar_read_size(struct lar *lar, int filelen) +{
- unsigned int *ptr = (unsigned int *) (lar->map + (filelen - 12));
- return ptr[0];
+}
What about all this pointer arithmetic? Is it really correct?
This code is not new, it is in another file already.
Doesn't adding 1 to a pointer actually add sizeof pointed-to-type to the address? Ie. this code isn't portable?
What makes you think so? It fetches an int from 12 bytes before the end of the file (ie. after the reset vector).
It is of course not portable because there is no reset vector at fffffff0 on non x86 machines. But that's another issue.
Then there's the matter of the pointer arithmetic again. Since ptr is uchar * this will work, but then writing size to p[0] will write a different number of bytes on different archs, right?
int is 32bit on all GNU architectures I know that could theoretically run LinuxBIOS. But since this is an x86 only thing anyways I would not care too much.
- lar = _open_lar(archive, s.st_size, O_RDWR);
Race conditions. First open() then fstat() to get the size.
Also, the file size can change even though we've opened the file.
Since the size is a rather important variable throughout lar we want to be able to handle it changing under our feet, but I don't know what the best way would be?
What kind of situation that realisticly happens would produce such an issue? i.e. While a payload builds a lar, or while LinuxBIOS is compiled?
- mkdirp((const char *) dirname(path), 0755);
- free(path);
- fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC, 0644);
- if (fd == -1) {
err("Error creating file %s\n", filename);return -1;- }
I don't think lar should mkdir -p implicitly. Just open() and perror() or print strerror(errno) in case of failure.
Oh it does not do this implicitly. It only does it if the archive explicitly contains a directory.
Stefan