Bug#731806: debian-installer: FTBFS on sparc: genisoimage errors

[Patrick Baggett <baggett.patrick@gmail.com>]

On Mon, Apr 28, 2014 at 12:25 PM, Thomas Schmitt <scdbackup@gmx.net> wrote:

Hi,

Patrick Baggett:

> Could you explain the context around this code? Perhaps the source is
> not really "alignment safe" and could use some patching upstream? I'd
> be happy to provide advice or code samples.

The context was misposted to bug report 731806 as message #87:
  https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=731806#87
(but not with Cc to debian-sparc list).

Upstream is myself. :))
The code in question is by my unreachable libburn predecessors, though.

It belongs to the preparation of a thread start for one of five
occasions in libburn. SIGBUS happens at
  http://libburnia-project.org/browser/libburn/trunk/libburn/async.c#L149

The caller has a local struct (i.e. on stack) like (#L592, #L692):

   struct write_opts write;
   ...
   add_worker(Burnworker_type_writE, d,
                   (WorkerFunc) write_disc_worker_func, &o);

The called function gets its address as parameter "data" (#L135):

  static void add_worker(int w_type, struct burn_drive *d,
                         WorkerFunc f, void *data)

has a struct on heap (#L102, #L138, #L146):

   struct w_list{
       ...
       union w_list_data
       {
           ...
           struct write_opts write;
           ...
       } u;
   }
   ...
   struct w_list *a;
   ...
   a = calloc(1, sizeof(struct w_list));

The gesture which causes the SIGBUS is (#L149)

   a->u = *(union w_list_data *)data;

which is not what i personally would use, but should be fully legal
nevertheless.

I'm not sure what the problem is exactly here. 64-bit SPARC will use ldx to load 64-bit quantities, which can cause problems if the source data is only 4-byte aligned, but that doesn't seem to be the case here. I really need a disassembly and to be able to probe the runtime stack a bit, so that really means that I need to build the code. :)

I think as a more meta-problem is this: the code's logic for "how much" to copy is wrong. It copies too many bytes on many cases and violates the C contract that you'll only copy like objects using "=".

Imagine:

sizeof(struct erase_ops) == 8 (32-bit ABI)

sizeof(w_list_data) == at least 12 (32-bit ABI).

Suppose: data = "">

a->u = *(union w_list_data *)data; // copies 12 bytes from 8 byte area, can cause crashes if the last 4 bytes are into an unmapped page.

The correct way to do this would be to have add_worker() pass a const w_list_data* with the appropriate field(s) in '.u' filled out. Otherwise, you're copying unlike objects of different sizes and that's never safe.

---

Patrick

 

The SIGBUS vanishes if i compile without gcc -O2, or if i replace
the "a->u =" gesture by

   memcpy(&(a->u), data, sizeof(union w_list_data));

which i deem equivalent (and more my personal style).

Have a nice day :)

Thomas