On Thu, Mar 17, 2022 at 09:02:46PM +0200, Jani Nikula wrote: > On Thu, 17 Mar 2022, Ville Syrjala <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > Make a copy of each VB data block with a guaranteed minimum > > size. The extra (if any) will just be left zeroed. > > *VBT > > > > > This means we don't have to worry about going out of bounds > > when accessing any of the structure members. Otherwise that > > could easliy happen if we simply get the version check wrong, > > or if the VBT is broken/malicious. > > *easily > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > The high level question is if we really want to save the copies until > driver remove instead of just during parsing. The lifetime should be > mentioned in the commit message, with rationale if you have some. Sure, I'll amend the commit msg a bit. I think we at least must move away from the "parse VBT once at driver init" idea because that won't ever let us deal with the panel_type=0xff->check PnP ID thing. I think I even have a few real world VBTs in my stash which have panel_type=0xff, so it's not just some theoretical thing. So we must hang onto the blocks at least until we've finished the output setup. But I'm thinking we can just keep them permanently and even start thinking about moving away from the "parse once, stash results somewhere" mentality. Ie. we could just parse on demand instead. > I was wondering about making the copies up front instead of as needed, > but that means setting up a list for the min sizes. It would clean up > the usage (avoids passing around any pointers to original data to the > parsers). Then you could use just find_section(i915, BDB_XXX). Dunno. At least if we go for the parse on demand apporach then we definitely want to do that. Avoids having to deal with kmalloc() fails/etc. while parsing. -- Ville Syrjälä Intel