On Mon, Apr 26, 2021 at 11:15:40PM +0200, Fabio M. De Francesco wrote:
> On Monday, April 26, 2021 9:21:01 PM CEST Matthew Wilcox wrote:
> > On Mon, Apr 26, 2021 at 08:42:45PM +0200, Fabio M. De Francesco wrote:
> > > +static void setup_scsitaskmgmt_handles(struct xarray *xa, struct
> uiscmdrsp *cmdrsp,
> > >
> > > wait_queue_head_t *event,
> int *result)
> > >
> > > {
> > >
> > > - /* specify the event that has to be triggered when this */
> > > - /* cmd is complete */
> > > - cmdrsp->scsitaskmgmt.notify_handle =
> > > - simple_idr_get(idrtable, event, lock);
> > > - cmdrsp->scsitaskmgmt.notifyresult_handle =
> > > - simple_idr_get(idrtable, result, lock);
> > > + u32 id;
> > > + int ret;
> > > +
> > > + /* specify the event that has to be triggered when this cmd is
> complete */
> > > + id = (u32)cmdrsp->scsitaskmgmt.notify_handle;
> > > + ret = xa_alloc_irq(xa, &id, event, XA_LIMIT(1, INT_MAX),
> GFP_KERNEL);
> >
> > OK, think this one through a bit. When xa_alloc_irq() stores the ID that
> > it assigned into 'id', what happens to it next?
> >
> Oh, I overlooked that... The ID in 'id' is lost when the function exits and
> the stack frame is unwound.
>
> Now I have another problem: xa_alloc_irq() writes id u32* but I have an u64*
> in 'id'. What happens if I cast 'id' to an u32* when passing it to
> xa_alloc_irq()?
>
> u64 *id;
> int ret;
> id = &cmdrsp->scsitaskmgmt.notify_handle;
> ret = xa_alloc_irq(xa, (u32 *)id, event, XA_LIMIT(1, INT_MAX), GFP_KERNEL);
>
> Do I destroy the information stored in 'id' with that cast?
That is a great question! That would be a really serious bug because
it behaves differently on big and little endian systems. That is, on a
little endian system, a pointer to a u64 can be treated as a pointer to a
u32 and it will write to the bottom 32 bits of the u64. On a big endian
system, treating a pointer to a u64 as if it's a pointer to a u32 means
you write to the _top_ 32 bits of the u64, and things go wrong from there!
Similarly, if you have a u16, you can't pass a pointer to it, because
the called function has no idea that it's only 16 bits, and will do a
32-bit store to it, overwriting the 16 bits after it.
So you need to pass a pointer to a u32 on the stack, and then copy the
id out of it afterwards.
