> > It's pretty "trivial" to look to see if the field is set in the pci_id > > structure, that should be all that is needed, right? > > Well... If you consider that all drivers using the driver_data field > should have the dynids.use_driver_data flag set unconditionally and > without looking at the code, then in fact you agree with Milton and > myself that this flag should be dropped. The whole point of the flag, > if my guess is correct, was to hint the developer that he/she should > double check his/her code before setting the flag. If you set it for > all these drivers, then all it does is preventing users from passing a > driver_data value to drivers which do _not_ use the driver_data... but > doing that has zero effect at the moment, so that wouldn't actually be > any different from dropping dynids.use_driver_data altogether. > > So I have to reiterate my support to Milton's idea of dropping > the dynids.use_driver_data flag. It does more harm than good. Yeah it doesn't seem to be very heavily used at this point. :) I see around 400 uses of id->driver_data right now, and only 2 of use_driver_data. If we remove the use_driver_data field, drivers (except for the 2 using the field) will start to see whatever values userspace passes to them rather than 0, and at least in some of the few cases I looked at that could cause breakage due to out of bounds references. So I think your point about dynamic IDs in general is a good one; this flag really does look like an "audit was done" bit, but doesn't go as far is it should. The patch you posted to forbid dynamic binding unless use_driver_data is iset is probably the safest thing to do, given that drivers that *don't* set use_driver_data look like they might misbehave even with a driver_data value of 0... What do you think, Greg? Convinced yet? :) Thanks, Jesse -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
