Messages in this thread | | | Subject | Re: [ 10/21] target: Add link_magic for fabric allow_link destination target_items | From | Ben Hutchings <> | Date | Sat, 19 Jan 2013 04:13:24 +0000 |
| |
On Fri, 2013-01-18 at 17:19 -0800, Greg Kroah-Hartman wrote: > 3.4-stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: Nicholas Bellinger <nab@linux-iscsi.org> > > commit 0ff8754981261a80f4b77db2536dfea92c2d4539 upstream. > > This patch adds [dev,lun]_link_magic value assignment + checks within generic > target_fabric_port_link() and target_fabric_mappedlun_link() code to ensure > destination config_item *target_item sent from configfs_symlink() -> > config_item_operations->allow_link() is the underlying se_device->dev_group > and se_lun->lun_group that we expect to symlink. [...] > --- a/drivers/target/target_core_fabric_configfs.c > +++ b/drivers/target/target_core_fabric_configfs.c > @@ -72,6 +72,12 @@ static int target_fabric_mappedlun_link( > struct se_portal_group *se_tpg; > struct config_item *nacl_ci, *tpg_ci, *tpg_ci_s, *wwn_ci, *wwn_ci_s; > int ret = 0, lun_access; > + > + if (lun->lun_link_magic != SE_LUN_LINK_MAGIC) { > + pr_err("Bad lun->lun_link_magic, not a valid lun_ci pointer:" > + " %p to struct lun: %p\n", lun_ci, lun); > + return -EFAULT; > + } > /* > * Ensure that the source port exists > */ [...]
This is absolutely not a valid way of doing dynamic type checking.
The problem is that these structures:
> --- a/include/target/target_core_base.h > +++ b/include/target/target_core_base.h > @@ -779,6 +779,8 @@ struct se_subsystem_dev { > }; > > struct se_device { > +#define SE_DEV_LINK_MAGIC 0xfeeddeef > + u32 dev_link_magic; > /* RELATIVE TARGET PORT IDENTIFER Counter */ > u16 dev_rpti_counter; > /* Used for SAM Task Attribute ordering */ > @@ -869,6 +871,8 @@ struct se_port_stat_grps { > }; > > struct se_lun { > +#define SE_LUN_LINK_MAGIC 0xffff7771 > + u32 lun_link_magic; > /* See transport_lun_status_table */ > enum transport_lun_status_table lun_status; > u32 lun_access;
have struct config_group embedded at different places. So you know where the magic number is for the expected type but if you get an object of the wrong type then you'll read some other area of memory that might just happen to have the expected magic number anyway.
It seems like you need a superclass for all the structures exposed through configfs, to hold the type information. I would suggest that you define something like:
struct se_config_group { struct config_group group; u32 magic; };
and put that in place of the plain struct config_group. You can then safely check the magic number before looking at anything in the larger structure.
Ben.
-- Ben Hutchings All the simple programs have been written, and all the good names taken. [unhandled content-type:application/pgp-signature] | |