From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f182.google.com (mail-pl1-f182.google.com [209.85.214.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3EB843DB62D for ; Mon, 30 Mar 2026 16:48:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.182 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774889306; cv=none; b=Z6NZ+1KweRFoM2HKJQity1HheJ7Ou3MAkZya2h7wP9SQkXZC096rIxf7IfmouwdfnTtch9eg76vNxRWT0B7GuusvyFMYOVCEGZ3A0MsHRb203sD+fFYlmvoCy/ApfmKQI3sQYTjIJTwUdGPNictt6dODJEIY4FgVJTlHbl7Swd8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774889306; c=relaxed/simple; bh=hen2AXtPUvgD73bDjGcI5nkrg5NaplFg2OY2iSU2ksc=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=hy8AvHWzSl9OjE3RoVskxfy58IAaKMo6dbyUvCAkzcLtAo9zHEmE2arYHlER4xBZ6lYymr+jQc3Ef3U9EmLbH4PCL4ITZmkVZ6Gy7SvJoCZkMLIa7HdgzbwT0oJzmpD11Hobc92xivBWvnXsGEh8L1+OTN05xkxKX+JK/UMEciA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=LvfqU1Y4; arc=none smtp.client-ip=209.85.214.182 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="LvfqU1Y4" Received: by mail-pl1-f182.google.com with SMTP id d9443c01a7336-2b2591757fbso4665ad.0 for ; Mon, 30 Mar 2026 09:48:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1774889304; x=1775494104; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=sG9DgsY3olsmyDsu0A4WoI8JUqxgp2EL2EN+xPxq65Q=; b=LvfqU1Y4axM4088+KDWchtmklPfAgyHgq/LEC64G9oOvrQJrSnY7qapOQXrX7XOHne dS0AqcsLbleX985KL3x1EjdgkRpAM4JQ1ewc224fJYo4ImuvPUnABOtqPFGlmHNVm1Ka gclLgfRYAps7acOfrrDLT/BTf9W8gyB5arpzSY1ZyP8+6uEm+xywUveLz0kRLYRGYuU0 5TRRx3AYZCRIub9zHHnAWlQ1yv4QGZ+K3lnERUSf986MZc5nj1eT+RIVcOGZTSCosW+W Le3dapxC/aIPjRioz9fESVZFiw15Dy+RmbdY6ZE1TYYvDlAdRhMfcXy4wmFsiBpSG7y8 0bqQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774889304; x=1775494104; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=sG9DgsY3olsmyDsu0A4WoI8JUqxgp2EL2EN+xPxq65Q=; b=YLOY0BxCL3+5OC4nfy/R2mQ/+evzSiVIN6yZaY+ED9fh6QfXIj2L1Lo5OfcoU2Dd7M gAHmSZittC0DNVCoqetnIlVDG0YdcWun94tK8zTwiubDSgcBj/uVkFs0Q294X72gDCRp aqG9UFVEKX59S7YV7y/Y4iC7Dp0g6tqOtyHe59mvIU6eD4HxOoAInhCLysNE0fgtUOlr MpZWksY1kVaHkCK1eFgDbWoAb4ObXLfKNhBLmI5kQ7zwtiB9WokyXHaZImjzQFQVAKIU sNKPRjH539WDbleDuVuJ0kjyQ25pwyJofAC6lhsEl9hEWhKdZenzEfZnmHbIMIuG2YRY PzUQ== X-Forwarded-Encrypted: i=1; AJvYcCU4KITc/FDDRJiH9QHA41P+moqmmqUXp/GjMmy/8eL86+Yd1/bbG4DMpOUnelL8M5p/ITfabLwJo0qfZ2E=@vger.kernel.org X-Gm-Message-State: AOJu0YxpNr0nryXkFq2lAo/evPZ5YTOx37F2+n2hhPGnlZYMW3pjmbTO DzzfQyj3JU/ooO+YHKnwaNG89UhZ9mTkL+PfHgZTu9D6BwbhuW/tQ4qoOZ0ezXwwkLbKLGwbe47 xPS9IxA== X-Gm-Gg: ATEYQzxwH+vEs4rYJkxGeQR4o1eHrwWhDIFjw6fKgin3H6Lk4/riFBVQmlLy7rsGSgj XoE3G32tIdrHJG+Y9iHqwVnnjDVfEGC6pTn1Et0qItz3Hha1diWh2De8IDT9x/BsJ8/20Y6m6sH 8Piw+K+pmqKGNFA26fc7yIu6F14SgIn3L97HhQbUKZTF85WQvLXbyWM9+EZz0Y5ntsjgma1PWUi GKHQ6DCzV/c5isqXrj4RX/cDw3cTI4GZE34ZgCHbL2Ee740WPDqD9OIrAs9cuIW9zIoyBxHh3ZY 4mut3udFSbOQbolMen1DCttHxiqc+vwhwbT2Sak9WVdkp/f/OV+d3Ax2qw+qJ1RnmlXLmtWFH7S vdfUtUnPONw+xia9dZHOUVOdNpFW6inK5n9TJ/sF36e4v/dC5AknJg3OW+P6DpcehQO6D7is5iz JE2RW3rXzasO4OILBoD+XsYUwdWWmIZ+jgZpZv+ZuO0wqh0W2xVvjkTSbus3vVBw== X-Received: by 2002:a17:902:f78a:b0:2b0:be7d:a25e with SMTP id d9443c01a7336-2b241d59313mr775145ad.18.1774889303958; Mon, 30 Mar 2026 09:48:23 -0700 (PDT) Received: from google.com (168.136.83.34.bc.googleusercontent.com. [34.83.136.168]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2b242676e13sm89196535ad.28.2026.03.30.09.48.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 30 Mar 2026 09:48:23 -0700 (PDT) Date: Mon, 30 Mar 2026 16:48:19 +0000 From: Samiullah Khawaja To: Pasha Tatashin Cc: rppt@kernel.org, akpm@linux-foundation.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, dmatlack@google.com, pratyush@kernel.org Subject: Re: [PATCH v3 03/10] liveupdate: Protect file handler list with rwsem Message-ID: References: <20260327033335.696621-1-pasha.tatashin@soleen.com> <20260327033335.696621-4-pasha.tatashin@soleen.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <20260327033335.696621-4-pasha.tatashin@soleen.com> On Fri, Mar 27, 2026 at 03:33:27AM +0000, Pasha Tatashin wrote: >Because liveupdate file handlers will no longer hold a module reference >when registered, we must ensure that the access to the handler list is >protected against concurrent module unloading. Nit: Here we make an assumption that the file (and flb) handler lifecycle is bound with the module lifecycle. It is a fair assumption, but maybe this can be documented somewhere? > >Utilize the global luo_register_rwlock to protect the global registry of >file handlers. Read locks are taken during list traversals in >luo_preserve_file() and luo_file_deserialize(). Write locks are taken >during registration and unregistration. > >Signed-off-by: Pasha Tatashin >--- > kernel/liveupdate/luo_core.c | 6 ++++++ > kernel/liveupdate/luo_file.c | 22 +++++++++++++++++----- > kernel/liveupdate/luo_internal.h | 2 ++ > 3 files changed, 25 insertions(+), 5 deletions(-) > >diff --git a/kernel/liveupdate/luo_core.c b/kernel/liveupdate/luo_core.c >index dda7bb57d421..f9ae9364a962 100644 >--- a/kernel/liveupdate/luo_core.c >+++ b/kernel/liveupdate/luo_core.c >@@ -54,6 +54,7 @@ > #include > #include > #include >+#include > #include > #include > #include >@@ -68,6 +69,11 @@ static struct { > u64 liveupdate_num; > } luo_global; > >+/* >+ * luo_register_rwlock - Protects registration of file handlers and FLBs. >+ */ >+DECLARE_RWSEM(luo_register_rwlock); >+ > static int __init early_liveupdate_param(char *buf) > { > return kstrtobool(buf, &luo_global.enabled); >diff --git a/kernel/liveupdate/luo_file.c b/kernel/liveupdate/luo_file.c >index a6d98fc75d25..4aea17a94b4f 100644 >--- a/kernel/liveupdate/luo_file.c >+++ b/kernel/liveupdate/luo_file.c >@@ -277,12 +277,14 @@ int luo_preserve_file(struct luo_file_set *file_set, u64 token, int fd) > goto err_fput; > > err = -ENOENT; >+ down_read(&luo_register_rwlock); > list_private_for_each_entry(fh, &luo_file_handler_list, list) { > if (fh->ops->can_preserve(fh, file)) { > err = 0; > break; > } > } >+ up_read(&luo_register_rwlock); We took the read lock here when running can_preserve, but then we use the fh without taking the lock later before calling file_preserve. This is safe since the module reference is taken and fh will not go away (based on the assumption I mentioned above). Maybe add a comment here that documents this assumption. > > /* err is still -ENOENT if no handler was found */ > if (err) >@@ -777,12 +779,14 @@ int luo_file_deserialize(struct luo_file_set *file_set, > bool handler_found = false; > struct luo_file *luo_file; > >+ down_read(&luo_register_rwlock); > list_private_for_each_entry(fh, &luo_file_handler_list, list) { > if (!strcmp(fh->compatible, file_ser[i].compatible)) { > handler_found = true; > break; > } > } >+ up_read(&luo_register_rwlock); > > if (!handler_found) { > pr_warn("No registered handler for compatible '%.*s'\n", >@@ -851,32 +855,36 @@ int liveupdate_register_file_handler(struct liveupdate_file_handler *fh) > if (!luo_session_quiesce()) > return -EBUSY; > >+ down_write(&luo_register_rwlock); > /* Check for duplicate compatible strings */ > list_private_for_each_entry(fh_iter, &luo_file_handler_list, list) { > if (!strcmp(fh_iter->compatible, fh->compatible)) { > pr_err("File handler registration failed: Compatible string '%s' already registered.\n", > fh->compatible); > err = -EEXIST; >- goto err_resume; >+ goto err_unlock; > } > } > > /* Pin the module implementing the handler */ > if (!try_module_get(fh->ops->owner)) { > err = -EAGAIN; >- goto err_resume; >+ goto err_unlock; > } > > INIT_LIST_HEAD(&ACCESS_PRIVATE(fh, flb_list)); > INIT_LIST_HEAD(&ACCESS_PRIVATE(fh, list)); > list_add_tail(&ACCESS_PRIVATE(fh, list), &luo_file_handler_list); >+ up_write(&luo_register_rwlock); >+ > luo_session_resume(); > > liveupdate_test_register(fh); > > return 0; > >-err_resume: >+err_unlock: >+ up_write(&luo_register_rwlock); > luo_session_resume(); > return err; > } >@@ -910,16 +918,20 @@ int liveupdate_unregister_file_handler(struct liveupdate_file_handler *fh) > if (!luo_session_quiesce()) > goto err_register; > >+ down_write(&luo_register_rwlock); > if (!list_empty(&ACCESS_PRIVATE(fh, flb_list))) >- goto err_resume; >+ goto err_unlock; > > list_del(&ACCESS_PRIVATE(fh, list)); >+ up_write(&luo_register_rwlock); >+ > module_put(fh->ops->owner); > luo_session_resume(); > > return 0; > >-err_resume: >+err_unlock: >+ up_write(&luo_register_rwlock); > luo_session_resume(); > err_register: > liveupdate_test_register(fh); >diff --git a/kernel/liveupdate/luo_internal.h b/kernel/liveupdate/luo_internal.h >index 8083d8739b09..4bfe00ac8866 100644 >--- a/kernel/liveupdate/luo_internal.h >+++ b/kernel/liveupdate/luo_internal.h >@@ -77,6 +77,8 @@ struct luo_session { > struct mutex mutex; > }; > >+extern struct rw_semaphore luo_register_rwlock; >+ > int luo_session_create(const char *name, struct file **filep); > int luo_session_retrieve(const char *name, struct file **filep); > int __init luo_session_setup_outgoing(void *fdt); >-- >2.43.0 >