Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Chuck Lever III <chuck.lever@oracle.com>
To:     Jeff Layton <jlayton@kernel.org>
CC:     Chuck Lever <cel@kernel.org>,
        Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: [PATCH v1 00/27] Server-side RPC reply header parsing overhaul
Thread-Topic: [PATCH v1 00/27] Server-side RPC reply header parsing overhaul
Thread-Index: AQHZI37m5Qh52doLRkW9UASLLYSz/a6Xv50AgAAGhIA=
Date:   Tue, 10 Jan 2023 15:16:50 +0000
Message-ID: <CF69C362-B07E-49C0-8DB4-D1DC029A824E@oracle.com>
References: <167319499150.7490.2294168831574653380.stgit@bazille.1015granger.net>
 <64b51ef5ed4b11f8a6d59bf59ce0ab8c36ef454f.camel@kernel.org>
In-Reply-To: <64b51ef5ed4b11f8a6d59bf59ce0ab8c36ef454f.camel@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?D9GjQwZ08GRlakqRx4cM4sabWDVAe9ErJTQxHGMjtNJuEOGuhzGJp3PDpMbg?=
 =?us-ascii?Q?YKVNJVBEEEkA/crb8gcTagPA1NIi0GwwOgnvIBYcR++dx2TDIrvSETJ9j+Bu?=
 =?us-ascii?Q?fFTFMNJSBdoSLnQObULmo/EX/ShRcBgrgbM+SUYnGYL9tMF2YAUvCGDGVjpk?=
 =?us-ascii?Q?JvKd4UNPbf/mKiw2n2ZpFkY5XNgRGPHOtzUUMxdE1Yy8BR5evYmrYsjXsysv?=
 =?us-ascii?Q?NIcZ1yHmm8940nD9YPhSUvWk8d7YRFCyHHjtZZbKzuWMq5Sl0WEfuuUZMmhr?=
 =?us-ascii?Q?8XPdKHe/J/xG7CNAHlmbxVOwClHB2BtPAKk076E/tfo4FNiAXJhxCu+re9jx?=
 =?us-ascii?Q?Itby3gaTeddlONBFOV8ZB/o83gifO2qbwZxWZzRs0zU5PneOSo3PGIyIEfbM?=
 =?us-ascii?Q?xuZZrmaSAgw2MuSyKBdpQjiJtzdJbrnhs8p1iVqHojJsTTsmuvNcSkXwoe7w?=
 =?us-ascii?Q?0x2ijKGU4tiQZhgQEX5vpBdAWldkc1Z9gyK2T2xsG4f9xGEKGwFQlRxcmsCG?=
 =?us-ascii?Q?tC1LBh1gMILZp2qrNoxx29jd4DihJ7zUPNgtyf3nKmIWVIGt9ot1HRgbXZOS?=
 =?us-ascii?Q?Dc0GGvHHJZCm6lit5NK3aqtzX+SQnvRZKSqyRtR/73BSVPdYiKvt7IX9aIBc?=
 =?us-ascii?Q?fVre19SfY+/W/rB4TBM5lcwKZSSClrZqj0Wx7hE+XrSnUrmY/G7pndPBWPOm?=
 =?us-ascii?Q?lsCin5NivKqzl1j3NePd5Q4IcxewfpS54XjjBDACZmBfS8fuO8XdRx2ht3US?=
 =?us-ascii?Q?XBj1w/AKRXw2PyePmCwJPpFJrNMNHgG17gverfQUWS4MJGqSluh+wmi/oqmO?=
 =?us-ascii?Q?im0aItoQFYA7qvbbzV9MYZLHoaul+8E4dNh/uV++vYJinn9Hz2OwEbDD8dI/?=
 =?us-ascii?Q?8Lld7oUgH21ftyPTEq7OdmLZznYWz64i8m2ilGkDj/nWRd6tlnypjG+Pp8+M?=
 =?us-ascii?Q?KGC8iEXQctHlDJezmV9iBGJk38yzWh7x4pbHDC23TGM3ORBMt6qiT7rGJ9V9?=
 =?us-ascii?Q?j9PPi89cLYvDPvQs0nPRHS2SoF+DUaLfziUAtalMkxbnbMvQvSPMvonMHpwy?=
 =?us-ascii?Q?PTfnvrlpIcHwDFaJ/fjB8soMNZc3OeTEUTa56mnsyL3muiik3ISLlGqlj3ea?=
 =?us-ascii?Q?MK/RhTkyylW7E1PF9zsMLdxTe690nBl6n+gAu8UFRNiqb0T9/uXQy+C7qLXl?=
 =?us-ascii?Q?wzVWTyiaCjK0ZKYkEi8Ujt/Anq6/RBuRY6fGlbsIX7BjZWWO2UtpEj4/KfKy?=
 =?us-ascii?Q?jFsgjaN8bm+MuUKOCERHX3scwdv8SK9K2pRwJo6BnUSp9rwff0Vp/QVClHSR?=
 =?us-ascii?Q?oYasEJoQEm/dHwNDosjCscXG/lkXJlYL7jKZWciQYI2jWgbXRmnaOOsWouiA?=
 =?us-ascii?Q?eCVQnSC1oyyCQCWg1D6EM7oIpmFdFxnEjgOSrGd+NOtycCMDYNj01yoD3L03?=
 =?us-ascii?Q?nEVgMiGxAi6VjyJydXEXLox7vwpB5YaCEcoamHnt/Jvw4x8rGIol2Ieq1xM+?=
 =?us-ascii?Q?zMxMd1l/8NLVFhQNqc1aAltUdlfGN1dyhC6Lvzw3lltnuVGdv5eS4NUXpWXZ?=
 =?us-ascii?Q?Q1ECtsLIHDPsSS7CrluesPbz3VuwxLQQsGKEQ9FZpEOrl0FaQLmdSZIFwdUK?=
 =?us-ascii?Q?og=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <6C86E82C50BE184FBE75970BF520750B@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: fmhDQ8AW4S3/nG80mVoC5CICvIAFLbuT9B1Cq5MyUV6nRmJKX6rKr8Tpf+JiE3AEMrYF2VovQQbJ94EAGcoM5tZ3K7acYMWVcHnc1lbxhXBwYSDtrS/pXCOMHQrw0eNVek7QoJNhH5tMdAiYt9x1IZw67Ze12x0GIz0ICYBggm458pratOTYymbfDU9uzZQrnMQV66F46sy7VHNcpbPcvW7FTVXt2rw8vr8zl2RmoVpqnOWabR0hFgWcUvbdz0J9WRbGFYD+6WEQkt5wD2Q9Zeu4sBoadWox2kr8tCpqOcFLBNv5ouC8gNPzHCZnTNdQ2j8YO5LVWh9wUGsReMC/7Re72Bq6WRWE/E23oarBssxTr+RteCDmj9x5zpwesejKWORNRnuGdU0MfouKsVlMrfupwV/576zth48+2t0OJfbG8Sbxoh8mzUVeZ/dqT7AeI/faYtyMBfNVwQv8QdfM5c2ruhPaRdXGaihQ5ZWPZYL8IjxXg0rkL9+SxrKej4CXcIdn8cXCkbKg+6oFFM7i4WmuDbLVPCsQhQELNavMNIThB+MuVMg6TdtEC1CYPfx49860DdD7r8bgN9ZvMwXkDOzp2odlai2SzRcU7Bp8CeY3NruZrpuf7nj02TVf9acJYuvmvfvClSbUdzDWk8pkI25BO80eRN0Hcm93OKfYLL9NcVTjXGQVkP+SE/M4Z0nAo+lbziww6mJxyggHGBTFCx6bbZCqFWBeU8NRD00hTwwMFdZ+SpnVm65GcQSct5mQ1h3AOEk0Np2eYbV+43tgmiZOGXsgl55W1rhDCgI3k1BK0DfJXB56nhStajSOuGXC
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN0PR10MB5128.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dbbd9754-8fda-4e4b-87c1-08daf31db287
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Jan 2023 15:16:50.0705
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: c0Shf0EWbxHq1h1sReEi9r0FURN7kNAK0E0oPBY6QCFxKXC/5FZLv6Ac+Mne68NcN/FaL2QU3FhVBZg6b7n/rw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR10MB7295
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2023-01-10_06,2023-01-10_03,2022-06-22_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=999 bulkscore=0 spamscore=0
 suspectscore=0 adultscore=0 phishscore=0 malwarescore=0 mlxscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000
 definitions=main-2301100095
X-Proofpoint-ORIG-GUID: puA42H72H2F2yCUDjfaw16mKiSu9mkuJ
X-Proofpoint-GUID: puA42H72H2F2yCUDjfaw16mKiSu9mkuJ
X-Spam-Status: No, score=-2.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org


> On Jan 10, 2023, at 9:53 AM, Jeff Layton <jlayton@kernel.org> wrote:
>=20
> On Sun, 2023-01-08 at 11:28 -0500, Chuck Lever wrote:
>> The purpose of this series is to replace the svc_put* macros in the
>> Linux kernel server's RPC reply header construction code with
>> xdr_stream helpers. I've measured no change in CPU utilization after
>> the overhaul.
>>=20
>> Memory safety: Buffer bounds checking after encoding each XDR item
>> is more memory-safe than the current mechanism. Subsequent memory
>> safety improvements to the common xdr_stream helpers will benefit
>> all who use them.
>>=20
>> Audit friendliness: The new code has additional comments and other
>> clean-up to help align it with the relevant RPC protocol
>> specifications. The use of common helpers also makes the encoders
>> easier to audit and maintain.
>>=20
>> I've split the full series in half to make it easier to review. The
>> patches posted here are the second half, handling RPC reply header
>> encoding.
>>=20
>> Note that another benefit of this work is that we are taking one or
>> two more strides closer to greater commonality between the client
>> and server implementations of RPCSEC GSS.
>>=20
>> ---
>>=20
>> Chuck Lever (27):
>>      SUNRPC: Clean up svcauth_gss_release()
>>      SUNRPC: Rename automatic variables in svcauth_gss_wrap_resp_integ()
>>      SUNRPC: Record gss_get_mic() errors in svcauth_gss_wrap_integ()
>>      SUNRPC: Replace checksum construction in svcauth_gss_wrap_integ()
>>      SUNRPC: Convert svcauth_gss_wrap_integ() to use xdr_stream()
>>      SUNRPC: Rename automatic variables in svcauth_gss_wrap_resp_priv()
>>      SUNRPC: Record gss_wrap() errors in svcauth_gss_wrap_priv()
>>      SUNRPC: Add @head and @tail variables in svcauth_gss_wrap_priv()
>>      SUNRPC: Convert svcauth_gss_wrap_priv() to use xdr_stream()
>>      SUNRPC: Check rq_auth_stat when preparing to wrap a response
>>      SUNRPC: Remove the rpc_stat variable in svc_process_common()
>>      SUNRPC: Add XDR encoding helper for opaque_auth
>>      SUNRPC: Push svcxdr_init_encode() into svc_process_common()
>>      SUNRPC: Move svcxdr_init_encode() into ->accept methods
>>      SUNRPC: Use xdr_stream to encode Reply verifier in svcauth_null_acc=
ept()
>>      SUNRPC: Use xdr_stream to encode Reply verifier in svcauth_unix_acc=
ept()
>>      SUNRPC: Use xdr_stream to encode Reply verifier in svcauth_tls_acce=
pt()
>>      SUNRPC: Convert unwrap data paths to use xdr_stream for replies
>>      SUNRPC: Use xdr_stream to encode replies in server-side GSS upcall =
helpers
>>      SUNRPC: Use xdr_stream for encoding GSS reply verifiers
>>      SUNRPC: Hoist init_encode out of svc_authenticate()
>>      SUNRPC: Convert RPC Reply header encoding to use xdr_stream
>>      SUNRPC: Final clean-up of svc_process_common()
>>      SUNRPC: Remove no-longer-used helper functions
>>      SUNRPC: Refactor RPC server dispatch method
>>      SUNRPC: Set rq_accept_statp inside ->accept methods
>>      SUNRPC: Go back to using gsd->body_start
>>=20
>>=20
>> fs/lockd/svc.c                    |   5 +-
>> fs/nfs/callback_xdr.c             |   6 +-
>> fs/nfsd/nfscache.c                |   4 +-
>> fs/nfsd/nfsd.h                    |   2 +-
>> fs/nfsd/nfssvc.c                  |  10 +-
>> include/linux/sunrpc/svc.h        | 116 +++----
>> include/linux/sunrpc/xdr.h        |  23 ++
>> include/trace/events/rpcgss.h     |  22 ++
>> net/sunrpc/auth_gss/svcauth_gss.c | 505 +++++++++++++++---------------
>> net/sunrpc/svc.c                  |  91 +++---
>> net/sunrpc/svcauth_unix.c         |  40 ++-
>> net/sunrpc/xdr.c                  |  29 ++
>> 12 files changed, 451 insertions(+), 402 deletions(-)
>>=20
>> --
>> Chuck Lever
>>=20
>=20
> I went through the whole set and this all looks like good stuff to me.
> The result is a lot more readable, and there is a lot less manual
> fiddling with buffer lengths and such.
>=20
> Do you have a public branch with the current state of this set?

These are in the topic-rpcsec-gss-krb5-enhancements branch in
my repo at kernel.org, although I'm about to push them to nfsd's
for-next.


> You can add:
>=20
> Reviewed-by: Jeff Layton <jlayton@kernel.org>

I very much appreciate your time!

--
Chuck Lever