Re: bug in zx's FBIOSCURSOR (with patch)

[Michael <macallan%netbsd.org@localhost>]

Hello,

On Sun, 10 Feb 2019 16:19:07 -0800
Jason Thorpe <thorpej%me.com@localhost> wrote:

I actually have a ZX ( actually a TurboZX, a two slot SBus monster that
came with two SBus cards that contain nothing but extra fans, so all in
all this thing occupies four(!) slots ) here, although I doubt a whole
lot of people are actually using that particular hw with netbsd...

> I was examining all of the uses of fubyte() in the kernel for reasons
> completely unrelated to SUNW,leo, but while doing to noticed that I
> think is a bug in that driver’s FBIOSCURSOR ioctl.
> 
> At the beginning of the block of code that handles that ioctl, a
> local variable v is set to the mask of cursor parameters to update,
> and is then tested in several subsequent blocks to determine what
> work is to be done.

I just looked at that code, ewwwwwww.....

> In the code that handles the FB_CUR_SETCMAP case, v is clobbered by
> several calls to fubyte().
> 
> This is fairly innocuous, unless you also happen to want to set the
> cursor shape in the same call, in which case the subsequent test to
> see if FB_CUR_SETSHAPE is set may fail as a result of v being
> clobbered while updating the cursor color map.
> 
> IMO, it’s just more straightforward to use copyin() for this, and so
> here is a patch that does so.  It compiles, but I can’t test it on
> real hardware.  If someone is able to do so, that’d be great.

I agree, that code looks highly suspicious.

> Related: the following block in zx_cursor_color() looks suspicious,
> so I’m calling it out here… but I don’t know enough about this
> particular bit of hardware to know for sure:
> 
>         tmp = sc->sc_curcmap[1] | (sc->sc_curcmap[3] << 8) |
>             (sc->sc_curcmap[5] << 16);
>         bus_space_write_4(sc->sc_bt, sc->sc_bhzcu, zcu_data,
> sc->sc_curcmap[1]);
> 
> (…really?  We’re not going to use “tmp” that we just went through all
> the trouble to calculate?)

That looks like a tpyo to me.

I'll dig up my SUNW,leo and see what this actually does. Somewhere I
even have docs...

have fun
Michael