From c6e80b4a7d3b86cae8b3552936c0a1b557cfa648 Mon Sep 17 00:00:00 2001 From: Sebastien Buisson Date: Thu, 19 Oct 2023 11:11:48 +0200 Subject: [PATCH] LU-17212 gss: survive improper obd or imp at ctx init GSS context init requests can happen even after a client has been unmounted, because they are coming from userspace (request-key, lgss_keyring). In this case they must be ignored, and code must be robust to survive improper, already or partially shutdown obd device or import. Lustre-change: https://review.whamcloud.com/52755 Lustre-commit: 3fcddf6dcdd92df6557c59913a61944f21d58615 Test-Parameters: kerberos=true testlist=sanity-krb5 Signed-off-by: Sebastien Buisson Change-Id: I541727165eadf1fcb7715e416da85d100976cf2f Reviewed-by: Aurelien Degremont Reviewed-by: Andreas Dilger Reviewed-on: https://review.whamcloud.com/c/ex/lustre-release/+/53291 Tested-by: jenkins Tested-by: Maloo --- lustre/ptlrpc/gss/gss_cli_upcall.c | 60 ++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 22 deletions(-) diff --git a/lustre/ptlrpc/gss/gss_cli_upcall.c b/lustre/ptlrpc/gss/gss_cli_upcall.c index 74d81e5..b37f043 100644 --- a/lustre/ptlrpc/gss/gss_cli_upcall.c +++ b/lustre/ptlrpc/gss/gss_cli_upcall.c @@ -223,13 +223,13 @@ int ctx_init_parse_reply(struct lustre_msg *msg, int swabbed, int gss_do_ctx_init_rpc(char __user *buffer, unsigned long count) { - struct obd_import *imp; - struct ptlrpc_request *req; - struct lgssd_ioctl_param param; - struct obd_device *obd; - char obdname[64]; - long lsize; - int rc; + struct obd_import *imp = NULL; + struct ptlrpc_request *req; + struct lgssd_ioctl_param param; + struct obd_device *obd; + char obdname[64]; + long lsize; + int rc; if (count != sizeof(param)) { CERROR("ioctl size %lu, expect %lu, please check lgss_keyring version\n", @@ -256,20 +256,30 @@ int gss_do_ctx_init_rpc(char __user *buffer, unsigned long count) obd = class_name2obd(obdname); if (!obd) { - CERROR("no such obd %s\n", obdname); - RETURN(-EINVAL); + rc = -EINVAL; + CERROR("%s: no such obd: rc = %d\n", obdname, rc); + RETURN(rc); } if (unlikely(!obd->obd_set_up)) { - CERROR("obd %s not setup\n", obdname); - RETURN(-EINVAL); + rc = -EINVAL; + CERROR("%s: obd not setup: rc = %d\n", obdname, rc); + RETURN(rc); } spin_lock(&obd->obd_dev_lock); if (obd->obd_stopping) { - CERROR("obd %s has stopped\n", obdname); + rc = -EINVAL; + CERROR("%s: obd has stopped: rc = %d\n", obdname, rc); spin_unlock(&obd->obd_dev_lock); - RETURN(-EINVAL); + RETURN(rc); + } + + if (!obd->obd_type || obd->obd_magic != OBD_DEVICE_MAGIC) { + rc = -EINVAL; + CERROR("%s: obd not valid: rc = %d\n", obdname, rc); + spin_unlock(&obd->obd_dev_lock); + RETURN(rc); } if (strcmp(obd->obd_type->typ_name, LUSTRE_MDC_NAME) && @@ -277,14 +287,16 @@ int gss_do_ctx_init_rpc(char __user *buffer, unsigned long count) strcmp(obd->obd_type->typ_name, LUSTRE_MGC_NAME) && strcmp(obd->obd_type->typ_name, LUSTRE_LWP_NAME) && strcmp(obd->obd_type->typ_name, LUSTRE_OSP_NAME)) { - CERROR("obd %s is not a client device\n", obdname); + rc = -EINVAL; + CERROR("%s: obd is not a client device: rc = %d\n", + obdname, rc); spin_unlock(&obd->obd_dev_lock); - RETURN(-EINVAL); + RETURN(rc); } spin_unlock(&obd->obd_dev_lock); down_read(&obd->u.cli.cl_sem); - if (obd->u.cli.cl_import == NULL) { + if (!obd->u.cli.cl_import || !obd->u.cli.cl_import->imp_obd || !obd->u.cli.cl_import->imp_sec) { CERROR("obd %s: import has gone\n", obd->obd_name); up_read(&obd->u.cli.cl_sem); RETURN(-EINVAL); @@ -293,22 +305,26 @@ int gss_do_ctx_init_rpc(char __user *buffer, unsigned long count) up_read(&obd->u.cli.cl_sem); if (imp->imp_deactive) { - CERROR("import has been deactivated\n"); + rc = -EINVAL; + CERROR("%s: import has been deactivated: rc = %d\n", + obd->obd_name, rc); class_import_put(imp); - RETURN(-EINVAL); + RETURN(rc); } req = ptlrpc_request_alloc_pack(imp, &RQF_SEC_CTX, LUSTRE_OBD_VERSION, SEC_CTX_INIT); - if (req == NULL) { + if (!req || !req->rq_cli_ctx || !req->rq_cli_ctx->cc_sec) { param.status = -ENOMEM; goto out_copy; } if (req->rq_cli_ctx->cc_sec->ps_id != param.secid) { - CWARN("original secid %d, now has changed to %d, cancel this negotiation\n", - param.secid, req->rq_cli_ctx->cc_sec->ps_id); - param.status = -EINVAL; + rc = -EINVAL; + CWARN("%s: original secid %d, now has changed to %d, cancel this negotiation: rc = %d\n", + obd->obd_name, param.secid, + req->rq_cli_ctx->cc_sec->ps_id, rc); + param.status = rc; goto out_copy; } -- 1.8.3.1