From: Chuck Lever <chuck.lever@oracle.com>
To: Bruce Fields <bfields@fieldses.org>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: fix_priv_head
Date: Fri, 24 Jul 2020 17:05:05 -0400 [thread overview]
Message-ID: <8005DB66-EC25-4DE2-847B-CEAD9BBE48E3@oracle.com> (raw)
In-Reply-To: <20200724203900.GB9244@fieldses.org>
> On Jul 24, 2020, at 4:39 PM, Bruce Fields <bfields@fieldses.org> wrote:
>
> On Fri, Jul 24, 2020 at 10:10:08AM -0400, Chuck Lever wrote:
>>>> I'd like to remove this code, but
>>>> I'd first like to understand how it will effect the code that follows
>>>> immediately after:
>>>>
>>>> offset = xdr_pad_size(buf->head[0].iov_len);
>>>> if (offset) {
>>>> buf->buflen = RPCSVC_MAXPAYLOAD;
>>>> xdr_shift_buf(buf, offset);
>>>> fix_priv_head(buf, pad);
>>>> }
>>
>> So if one of those patches removes "pad = priv_len - buf->len;"
>> then the above code will break.
>>
>> But I'm trying to see when it is possible for gss_unwrap to
>> return a head buffer that is not quad-aligned. Not coming up
>> with any such scenario.
>
> Thinking about it more, even if there was some gss mechanism returning
> misaligned data, the best place to fix that would likely be in the
> mechanism-specific code (partly for reasons noted in the comment right
> here--it'll be more efficient to put the data in the right spot as you
> encrypt it.)
In principal, I totally agree that the GSS mechanism's unwrap method is
the obvious place to handle mis-alignment. The practical challenge in
this code path is that the needs of the client and server receive logic
diverge just enough to make it annoying.
So another remark about this:
static void
fix_priv_head(struct xdr_buf *buf, int pad)
{
if (buf->page_len == 0) {
/* We need to adjust head and buf->len in tandem in this
* case to make svc_defer() work--it finds the original
* buffer start using buf->len - buf->head[0].iov_len. */
buf->head[0].iov_len -= pad;
}
}
The comment complains about svc_defer, and that particular calculation
is still in that code. It seems like it would be better if a pointer
into buf->head was saved somewhere instead of trying to manufacture
it based on buf->len, which seems to be pretty unreliable.
If svc_defer was changed that way, that might help us get rid of at
least the first fix_priv_head call site.
--
Chuck Lever
prev parent reply other threads:[~2020-07-24 21:05 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-23 17:46 fix_priv_head Chuck Lever
2020-07-23 19:38 ` fix_priv_head Bruce Fields
2020-07-23 20:23 ` fix_priv_head Chuck Lever
2020-07-24 1:17 ` fix_priv_head Bruce Fields
2020-07-24 14:10 ` fix_priv_head Chuck Lever
2020-07-24 20:39 ` fix_priv_head Bruce Fields
2020-07-24 21:05 ` Chuck Lever [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8005DB66-EC25-4DE2-847B-CEAD9BBE48E3@oracle.com \
--to=chuck.lever@oracle.com \
--cc=bfields@fieldses.org \
--cc=linux-nfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).