staging: zcache: rename ramster to zcache

Dan Magenheimer dan.magenheimer at oracle.com
Tue Apr 30 20:15:56 UTC 2013


> From: Dan Carpenter
> Subject: re: staging: zcache: rename ramster to zcache
> 
> Hi Dan,
> 
> I had a question about 703ba7fe5e08: "staging: zcache: rename ramster
> to zcache" from Jan 18, 2013.
> 
> We call ramster_remote_put_handler() from r2net_process_message() like
> this:
> 
> drivers/staging/zcache/ramster/tcp.c
>   1320          if (be16_to_cpu(hdr->data_len) > nmh->nh_max_len)
>   1321                  syserr = R2NET_ERR_OVERFLOW;
>   1322
>   1323          if (syserr != R2NET_ERR_NONE) {
>   1324                  pr_err("ramster_r2net, message length problem\n");
>   1325                  goto out_respond;
>   1326          }
>   1327
>   1328          r2net_set_func_start_time(sc);
>   1329          sc->sc_msg_key = be32_to_cpu(hdr->key);
>   1330          sc->sc_msg_type = be16_to_cpu(hdr->msg_type);
>   1331          handler_status = (nmh->nh_func)(hdr, sizeof(struct r2net_msg) +
>   1332                                               be16_to_cpu(hdr->data_len),
>   1333                                          nmh->nh_func_data, &ret_data);
> 
> "len" here is "sizeof(struct r2net_msg) + be16_to_cpu(hdr->data_len)",
> in other words it's a number from 24-65559.
> 
> drivers/staging/zcache/ramster/r2net.c
>    110  int ramster_remote_put_handler(struct r2net_msg *msg,
>    111                                  u32 len, void *data, void **ret_data)
>    112  {
>    113          struct tmem_xhandle *xh;
>    114          char *p = (char *)msg->buf;
>    115          int datalen = len - sizeof(struct r2net_msg) -
>    116                                  sizeof(struct tmem_xhandle);
> 
> If len is 24 then we're setting "datalen" to a negative number.  I
> followed the code and I think there is a path where a negative here
> might trigger a BUG_ON().
> 
>    117          u16 msgtype = be16_to_cpu(msg->msg_type);
>    118          bool ephemeral = (msgtype == RMSTR_TMEM_PUT_EPH);
>    119          unsigned long flags;
>    120          int ret;
>    121
>    122          xh = (struct tmem_xhandle *)p;
>    123          p += sizeof(struct tmem_xhandle);
>    124          zcache_autocreate_pool(xh->client_id, xh->pool_id, ephemeral);
>    125          local_irq_save(flags);
>    126          ret = zcache_put_page(xh->client_id, xh->pool_id, &xh->oid, xh->index,
>    127                                  p, datalen, true, ephemeral);
>    128          local_irq_restore(flags);
>    129          return ret;
>    130  }
> 
> regards,
> dan carpenter

Hi Dan --

Cc'ing Konrad.

Thanks for reviewing at the code.

I don't think the case you point out can ever occur as both the
data and the len always include a struct tmem_xhandle, thus len
is always greater than or equal to sizeof(struct r2net_message) PLUS
sizeof(struct tmem_xhandle).  Note the hardcoded
"p += sizeof(struct tmem_xhandle);" that skips over it in
the data.

So I don't think it is a bug, but it could probably use some
additional commenting.

Dan



More information about the devel mailing list