BUG? in mkswap (Re: The current status of sebusybox project)

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

KaiGai Kohei wrote:
> Karel,
> 
> Can I consider that you are the most appropriate person to report
> about the following matter?
> 
> The changelog in util-linux-ng.spec says:
> | * Wed Mar  8 2006 Karel Zak <kzak at redhat.com> 2.13-0.17
> | - fix #181782 - mkswap selinux relabeling (fix util-linux-2.13-mkswap-selinux.patch)
> 
>>  * /sbin/mkswap (should be ported later.)
>>   - It enables to relabel the target file as "swapfile_t", when we use
>>     a regular file as a swap.
> 
> In util-linux-ng-2.13-1.fc8.src.rpm, this feature is implemented
> as follows:
> 
> at util-linux-ng-2.13/disk-utils/mkswap.c
> -------------------------------------------------------
>     75  #define SELINUX_SWAPFILE_TYPE   "swapfile_t"
>     :                :
>    735  #ifdef HAVE_LIBSELINUX
>    736          if (S_ISREG(statbuf.st_mode) && is_selinux_enabled()) {
>    737                  security_context_t context_string;
>    738                  security_context_t oldcontext;
>    739                  context_t newcontext;
>    740
>    741                  if ((fgetfilecon(DEV, &oldcontext) < 0) &&
>    742                      (errno != ENODATA)) {
>    743                          fprintf(stderr, _("%s: %s: unable to obtain selinux file label: %s\n"),
>    744                                          program_name, device_name,
>    745                                          strerror(errno));
>    746                          exit(1);
>    747                  }
>    748                  if (!(newcontext = context_new(oldcontext)))
>    749                          die(_("unable to create new selinux context"));
>    750                  if (context_type_set(newcontext, SELINUX_SWAPFILE_TYPE))
>    751                          die(_("couldn't compute selinux context"));
>    752
>    753                  context_string = context_str(newcontext);
>    754
>    755                  if (strcmp(context_string, oldcontext)!=0) {
>    756                          if (fsetfilecon(DEV, context_string)) {
>    757                                  fprintf(stderr, _("%s: unable to relabel %s to %s: %s\n"),
>    758                                                  program_name, device_name,
>    759                                                  context_string,
>    760                                                  strerror(errno));
>    761                                  exit(1);
>    762                          }
>    763                  }
>    764                  context_free(newcontext);
>    765                  freecon(oldcontext);
>    766          }
>    767  #endif
> -------------------------------------------------------
> 
> Pay attention around line 741.
> If fgetfilecon() fails and returns -ENODATA, context_new() will be
> called with uninitialized oldcontext in the next. Then, it cause
> a segmentation fault.
> 
> If you don't want to exit immediately, I think this logic can be
> changed as follows:
> -------------------------------------------------------
>     if (fgetfilecon(DEV, &oldcontext) < 0) {
>         if (errno != ENODATA) {
>             fprintf(stderr, _("%s: %s: unable to obtain selinux file label: %s\n"),
>                             program_name, device_name,
>                             strerror(errno));
>             exit(1);
>         }
>         if (matchpathcon(devide_name, statbuf.st_mode, &oldcontext))
>             die(_("unable to create new selinux context"));
>     }
>     if (!(newcontext = context_new(oldcontext)))
>         die(_("unable to create new selinux context"));
> -------------------------------------------------------
> 
> Thanks,
Please open a bugzilla.  Too easy to lease in email.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFHD3lJrlYvE4MpobMRAuEsAJ47goFYNYHeuFgqES9Tjns+5/J29gCdE5j+
hUHoowS2WpBJsNU09/4ZaDA=
=kC64
-----END PGP SIGNATURE-----