On Thu, Feb 11, 2021 at 9:45 AM Ben Widawsky <ben.widawsky@xxxxxxxxx> wrote: [..] > > > + if (mbox_cmd.size_out > sizeof(gsl)) { > > > + dev_warn(dev, "%zu excess logs\n", > > > + (mbox_cmd.size_out - sizeof(gsl)) / > > > + sizeof(struct gsl_entry)); > > > > This could well happen given spec seems to allow for other > > entries defined by other specs. > > Interesting. When I read the spec before (multiple times) I was certain it said > other UUIDs aren't allowed. You're correct though that the way it is worded, > this is a bad check. AIUI, the spec permits any UUID and as such I think we > should remove tainting for unknown UUIDs. Let me put the exact words: > > Table 169 & 170 > "Log Identifier: UUID representing the log to retrieve data for. The following > Log Identifier UUIDs are defined in this specification" > > To me this implies UUIDs from other (not "this") specifications are permitted. > > Dan, I'd like your opinion here. I'm tempted to change the current WARN to a > dev_dbg or somesuch. Yeah, sounds ok, and the command is well defined to be a read-only, zero-side-effect affair. If a vendor did really want to sneak in a proprietary protocol over this interface it would be quite awkward.