[PATCH v2]: write SELinux contexts when extracting a tar file
Denys Vlasenko
vda.linux at googlemail.com
Tue Mar 16 15:52:18 UTC 2010
On Tue, Mar 16, 2010 at 3:58 PM, J. Tang <tang at jtang.org> wrote:
> Here is a second iteration of the patch to enhanch tar, incorporating
> suggestions be Denys. It is baselined against the most recent commit
> to origin/master.
pax doc says:
> + /* forcibly add a newline at the end of the buffer, to prevent
> + any buffer overflows */
> + p = buf = xmalloc(sz);
> + buf[sz - 1] = '\n';
> +
> + xread(archive_handle->src_fd, buf, sz);
Look carefully. You stored '\n' in buf[sz - 1].
The very next command reads sz bytes, thus fills
buf[0...sz - 1], destroying '\n'.
> + do {
> + /* skip leading whitespace */
> + while (*p == ' ' || *p == '\t') {
> + p++;
> + }
Why do you skip it?
http://www.opengroup.org/onlinepubs/009695399/utilities/pax.html
"pax Extended Header" section, does not say there can be leading whitespace.
This is what I saw in linux kernel tarball:
00000200 35 32 20 63 6f 6d 6d 65 6e 74 3d 62 31 30 35 30 |52 comment=b1050|
00000210 32 62 32 32 61 31 32 30 39 64 36 62 34 37 36 33 |2b22a1209d6b4763|
00000220 39 64 38 38 62 38 31 32 62 32 31 66 62 35 39 34 |9d88b812b21fb594|
00000230 39 65 34 0a 00 00 00 00 00 00 00 00 00 00 00 00 |9e4.............|
No leading whitespace, as you see.
Do you have an example tar file where it exists?
> + /* scan for the length of the record */
> + len = bb_strtou(p, &keyword, 10);
> + /* expect errno to be EINVAL, because the character
> + following the digits is supposed to be a space */
> + if ((errno != EINVAL) || ((p + len) > (buf + sz))) {
|| len == 0 || *keyword != ' '
> + bb_error_msg("Malformed extended header: length is out of allowed range");
> + return;
> + }
> +
> + next_p = p + len;
> +
> + p = keyword;
> +
> + /* scan for the start of the keyword and the value;
> + they are '=' separated */
> +
> + while (*p == ' ' || *p == '\t') {
> + p++;
> + }
> + keyword = p;
> +
> + while (*p != '=' && *p != '\n') {
> + p++;
> + }
> + if (*p != '=') {
> + bb_error_msg("Malformed extended header: missing equal sign");
> + return;
> + }
> +
> + *p = '\0';
> +
> + p++;
> + value = p;
> + while (*p != '\n') {
> + p++;
> + }
> + *p = '\0';
> +
> + p = next_p;
> +
> + /* handle fields that are SELinux related */
> + if (strcmp(keyword, SELINUX_CONTEXT_KEYWORD) == 0) {
> + /* free old context, in case context is given
> + multiple times */
*** where is the "free old context" operation mentioned in the comment?
> + if (setfscreatecon(value) < 0) {
> + bb_perror_msg("cannot set label to '%s'", value);
> + }
> + }
All of the above can be replaced by simpler code like this:
p += len;
p[-1] = '\0'; /* replace terminating '\n' */
if (strncmp(keyword+1, SELINUX_CONTEXT_KEYWORD"=",
sizeof(SELINUX_CONTEXT_KEYWORD"=")-1) == 0) {
/* We found it */
value = keyword + sizeof(SELINUX_CONTEXT_KEYWORD"=");
if (setfscreatecon(value) < 0) {
bb_perror_msg("can't set label to '%s'", value);
}
}
> + } while (p < buf + sz);
--
vda
More information about the busybox
mailing list