[PATCH 2/4] Add GPIO ioctl decoding

[Dmitry V. Levin]

On Sat, Jan 02, 2021 at 11:44:03PM +0800, Kent Gibson wrote:
> On Wed, Dec 30, 2020 at 12:47:28PM +0300, Dmitry V. Levin wrote:
> > On Wed, Dec 30, 2020 at 10:58:14AM +0800, Kent Gibson wrote:
> > > On Wed, Dec 30, 2020 at 05:07:42AM +0300, Dmitry V. Levin wrote:
> > > > On Wed, Dec 23, 2020 at 06:07:46AM +0800, Kent Gibson wrote:
> > > > > Decode the GPIO character device ioctls first introduced in Linux v4.8,
> > > > > as well as those added in subsequent kernel releases up to Linux v5.7.
> > > > > 
> > > > > Signed-off-by: Kent Gibson <warthog618 at gmail.com>
> > > > [...]
> > > 
> > > [...]
> > > > > +	PRINT_FIELD_U("{", info, line_offset);
> > > > > +	if (!tcp->u_rval) {
> > > > > +		tprints(", flags=");
> > > > > +		printflags(gpio_line_flags, info.flags, "GPIOLINE_FLAG_???");
> > > > > +		PRINT_FIELD_CSTRING(", ", info, name);
> > > > > +		PRINT_FIELD_CSTRING(", ", info, consumer);
> > > > > +	}
> > > > > +	tprints("}");
> > > > 
> > > > Both GPIO_GET_LINEINFO_IOCTL and GPIO_GET_LINEINFO_WATCH_IOCTL are
> > > > read-write ioctls, that is, the kernel reads struct gpioline_info on
> > > > entering syscall and writes it back on exiting.  A comprehensive decoder
> > > > would do the same, i.e. print the relevant parts of the structure both on
> > > > entering and on exiting.  There would be no need to use
> > > > umove_or_printaddr_ignore_syserror and ignore syscall errors.
> > > > 
> > > 
> > > Sorry I don't have prompt replies for your other comments - this code has
> > > been sitting in my working tree for months as I was having problems
> > > mailing to the list back then so I'll have to spend some time recapturing
> > > what I was thinking while writing it.
> > > 
> > > A decoder can't be comprehensive if it is only decoding relevant parts ;-).
> > > 
> > > So should the decoder decode the full struct both ways, or just the fields
> > > the kernel reads on the way in and the fields it wrote on the way out?
> > 
> > There is no need to print the full struct two times, only those fields
> > that could be accessed by the kernel.
> 
> Is this the sort of thing you are after for the IOWRs?:
> 
> [pid   107] ioctl(3, GPIO_GET_LINEINFO_IOCTL, {line_offset=2} => {flags=GPIOLINE_FLAG_KERNEL, name="gpio-mockup-A-2", consumer="test-as-is"}) = 0
> 
> I'm using this idiom to decode it:
> 
> static int
> print_gpioline_info(struct tcb *const tcp, const kernel_ulong_t arg)
> {
> 	struct gpioline_info info;
> 
> 	if (entering(tcp)) {
> 		tprints(", ");
> 		if (umove_or_printaddr(tcp, arg, &info))
> 			return RVAL_IOCTL_DECODED;
> 
> 		PRINT_FIELD_U("{", info, line_offset);
> 		tprints("}");
> 		return 0;
> 	}
> 
> 	/* exiting */
>     if (!syserror(tcp) && !umove(tcp, arg, &info)) {
> 		tprints(" => {flags=");
> 		printflags(gpio_line_flags, info.flags, "GPIOLINE_FLAG_???");
> 		PRINT_FIELD_CSTRING(", ", info, name);
> 		PRINT_FIELD_CSTRING(", ", info, consumer);
> 		tprints("}");
> 	}
> 
> 	return RVAL_IOCTL_DECODED;
> }

Yes, and you can use PRINT_FIELD_FLAGS macro as well.

> If that works for you I'll rework the rest similarly.

Thanks,


-- 
ldv