Re: [PATCH 2/2] SUNRPC: Rename xdr_buf_read_netobj to xdr_buf_read_mic

Benjamin Coddington
2019-09-13T17:26:11Z

On 13 Sep 2019, at 11:16, Chuck Lever wrote:

>> On Sep 12, 2019, at 1:07 PM, Benjamin Coddington
>> <bcodding@redhat.com> wrote:
>>
>> Let the name reflect the single user and purpose.
>
> And perhaps the assumption that the MIC is always the last item in the
> xdr_buf?
> I'm still reviewing 1/2, but this function might make that assumption.

It does.. is that a bad assumption, or maybe you'd like a different
name?

>> Signed-off-by: Benjamin Coddington <bcodding@redhat.com>
>> ---
>> include/linux/sunrpc/xdr.h | 2 +-
>> net/sunrpc/auth_gss/auth_gss.c | 2 +-
>> net/sunrpc/xdr.c | 42
>> +++++++++++++++++-----------------
>> 3 files changed, 23 insertions(+), 23 deletions(-)
>>
>> diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
>> index 9ee3970ba59c..a6b63e47a79b 100644
>> --- a/include/linux/sunrpc/xdr.h
>> +++ b/include/linux/sunrpc/xdr.h
>> @@ -179,7 +179,7 @@ xdr_adjust_iovec(struct kvec *iov, __be32 *p)
>> extern void xdr_shift_buf(struct xdr_buf *, size_t);
>> extern void xdr_buf_from_iov(struct kvec *, struct xdr_buf *);
>> extern int xdr_buf_subsegment(struct xdr_buf *, struct xdr_buf *,
>> unsigned int, unsigned int);
>> -extern int xdr_buf_read_netobj(struct xdr_buf *, struct xdr_netobj
>> *, unsigned int);
>> +extern int xdr_buf_read_mic(struct xdr_buf *, struct xdr_netobj *,
>> unsigned int);
>> extern int read_bytes_from_xdr_buf(struct xdr_buf *, unsigned int,
>> void *, unsigned int);
>> extern int write_bytes_to_xdr_buf(struct xdr_buf *, unsigned int,
>> void *, unsigned int);
>>
>> diff --git a/net/sunrpc/auth_gss/auth_gss.c
>> b/net/sunrpc/auth_gss/auth_gss.c
>> index 4ce42c62458e..d75fddca44c9 100644
>> --- a/net/sunrpc/auth_gss/auth_gss.c
>> +++ b/net/sunrpc/auth_gss/auth_gss.c
>> @@ -1960,7 +1960,7 @@ gss_unwrap_resp_integ(struct rpc_task *task,
>> struct rpc_cred *cred,
>>
>> if (xdr_buf_subsegment(rcv_buf, &integ_buf, data_offset, integ_len))
>> goto unwrap_failed;
>> - if (xdr_buf_read_netobj(rcv_buf, &mic, mic_offset))
>> + if (xdr_buf_read_mic(rcv_buf, &mic, mic_offset))
>> goto unwrap_failed;
>> maj_stat = gss_verify_mic(ctx->gc_gss_ctx, &integ_buf, &mic);
>> if (maj_stat == GSS_S_CONTEXT_EXPIRED)
>> diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
>> index 6e05a9693568..90dfde50f0ef 100644
>> --- a/net/sunrpc/xdr.c
>> +++ b/net/sunrpc/xdr.c
>> @@ -1236,52 +1236,52 @@ xdr_encode_word(struct xdr_buf *buf, unsigned
>> int base, u32 obj)
>> }
>> EXPORT_SYMBOL_GPL(xdr_encode_word);
>>
>> -/* If the netobj starting offset bytes from the start of xdr_buf is
>> contained
>> - * entirely in the head, pages, or tail, set object to point to it;
>> otherwise
>> - * shift the buffer until it is contained entirely within the pages
>> or tail.
>> +/* If the mic starting offset bytes from the start of xdr_buf is
>> contained
>> + * entirely in the head, pages, or tail, set mic to point to it;
>> otherwise
>> + * shift the buf until it is contained entirely within the pages or
>> tail.
>> */
>
> Nit: Could the patch convert this into a kernel Doxygen style comment?

Yes, can do that.

Ben

>> -int xdr_buf_read_netobj(struct xdr_buf *buf, struct xdr_netobj *obj,
>> unsigned int offset)
>> +int xdr_buf_read_mic(struct xdr_buf *buf, struct xdr_netobj *mic,
>> unsigned int offset)
>> {
>> struct xdr_buf subbuf;
>> unsigned int len_to_boundary;
>>
>> - if (xdr_decode_word(buf, offset, &obj->len))
>> + if (xdr_decode_word(buf, offset, &mic->len))
>> return -EFAULT;
>>
>> offset += 4;
>>
>> - /* Is the obj partially in the head? */
>> + /* Is the mic partially in the head? */
>> len_to_boundary = buf->head->iov_len - offset;
>> - if (len_to_boundary > 0 && len_to_boundary < obj->len)
>> + if (len_to_boundary > 0 && len_to_boundary < mic->len)
>> xdr_shift_buf(buf, len_to_boundary);
>>
>> - /* Is the obj partially in the pages? */
>> + /* Is the mic partially in the pages? */
>> len_to_boundary = buf->head->iov_len + buf->page_len - offset;
>> - if (len_to_boundary > 0 && len_to_boundary < obj->len)
>> + if (len_to_boundary > 0 && len_to_boundary < mic->len)
>> xdr_shrink_pagelen(buf, len_to_boundary);
>>
>> - if (xdr_buf_subsegment(buf, &subbuf, offset, obj->len))
>> + if (xdr_buf_subsegment(buf, &subbuf, offset, mic->len))
>> return -EFAULT;
>>
>> - /* Most likely: is the obj contained entirely in the tail? */
>> - obj->data = subbuf.tail[0].iov_base;
>> - if (subbuf.tail[0].iov_len == obj->len)
>> + /* Most likely: is the mic contained entirely in the tail? */
>> + mic->data = subbuf.tail[0].iov_base;
>> + if (subbuf.tail[0].iov_len == mic->len)
>> return 0;
>>
>> - /* ..or is the obj contained entirely in the head? */
>> - obj->data = subbuf.head[0].iov_base;
>> - if (subbuf.head[0].iov_len == obj->len)
>> + /* ..or is the mic contained entirely in the head? */
>> + mic->data = subbuf.head[0].iov_base;
>> + if (subbuf.head[0].iov_len == mic->len)
>> return 0;
>>
>> - /* obj is in the pages: move to tail */
>> - if (obj->len > buf->buflen - buf->len)
>> + /* mic is in the pages: move to tail */
>> + if (mic->len > buf->buflen - buf->len)
>> return -ENOMEM;
>> - obj->data = buf->head[0].iov_base + buf->head[0].iov_len;
>> - __read_bytes_from_xdr_buf(&subbuf, obj->data, obj->len);
>> + mic->data = buf->head[0].iov_base + buf->head[0].iov_len;
>> + __read_bytes_from_xdr_buf(&subbuf, mic->data, mic->len);
>>
>> return 0;
>> }
>> -EXPORT_SYMBOL_GPL(xdr_buf_read_netobj);
>> +EXPORT_SYMBOL_GPL(xdr_buf_read_mic);
>>
>> /* Returns 0 on success, or else a negative error code. */
>> static int
>> --
>> 2.20.1
>>
>
> --
> Chuck Lever