On Wed, 2013-08-14 at 19:41 +0200, Michal Židek wrote:
+ strs_offset = offsetof(struct sss_mc_grp_data, strs);
data = (struct sss_mc_grp_data *)rec->data;
+ /* Integrity check
+ * - name_len cannot be longer than all strings
+ * - data->name cannot point outside strings
+ * - all strings must be within data_table */
+ if (name_len > data->strs_len
+ || data->name > strs_offset + data->strs_len
+ || (uint8_t *)data->strs + data->strs_len > max_addr) {
+ ret = ENOENT;
+ goto done;
+ }
+
Sorry Michal, I noticed this before but forgot to comment on it.
I think the second check should be:
|| (data->name + name_len) > (strs_offset + data->strs_len)
Otherwise you could run out of bounds in the strcmp.
Also given this same complex check is done in 2 places maybe it can make
sense to have a common utility function or a macro to do the check.
The other patches look good.
Simo.
--
Simo Sorce * Red Hat, Inc * New York