From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754824AbbKLPDt (ORCPT ); Thu, 12 Nov 2015 10:03:49 -0500 Received: from mx2.suse.de ([195.135.220.15]:42374 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751048AbbKLPDs (ORCPT ); Thu, 12 Nov 2015 10:03:48 -0500 Date: Thu, 12 Nov 2015 16:03:45 +0100 From: Petr Mladek To: Miroslav Benes Cc: Jessica Yu , Rusty Russell , Josh Poimboeuf , Seth Jennings , Jiri Kosina , Vojtech Pavlik , linux-api@vger.kernel.org, live-patching@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org Subject: Re: module: save load_info for livepatch modules Message-ID: <20151112150345.GT2599@pathway.suse.cz> References: <1447130755-17383-1-git-send-email-jeyu@redhat.com> <1447130755-17383-3-git-send-email-jeyu@redhat.com> <20151112053228.GD30025@packer-debian-8-amd64.digitalocean.com> <20151112102404.GL4431@pathway.suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu 2015-11-12 14:22:28, Miroslav Benes wrote: > On Thu, 12 Nov 2015, Petr Mladek wrote: > > > >Maybe I am missing something but isn't it necessary to call vfree() on > > > >info somewhere in the end? > > > > > > So free_copy() will call vfree(info->hdr), except in livepatch modules > > > we want to keep all the elf section information stored there, so we > > > avoid calling free_copy(), As for the info struct itself, if you look > > > at the init_module and finit_module syscall definitions in > > > kernel/module.c, you will see that info is actually a local function > > > variable, simply passed in to the call to load_module(), and will be > > > automatically deallocated when the syscall returns. :-) No need to > > > explicitly free info. > > > > We still have to free the copied or preserved structures when > > the module is unloaded. > > ...freeing what we allocated. We need to free info->hdr somewhere if not > here and also mod_arch_specific struct where the patch module is removed. > This would unfortunately lead to changes in arch-specific code in > module.c. For example in arch/s390/kernel/module.c there is vfree call on > part of mod_arch_specific in module_finalize. We would call it only if the > flag mentioned above is not set and at the same time we would need to call > it when the patch module is being removed. Sigh, I am afraid that the flag is not enough. IMHO, we need to split the load finalizing functions into two pieces. One will be always called when the module load is finalized. The other part will free the load_info. It will be called either when the load is finalized or when the module is unloaded, depending on if we want to preserve the load_info. Sigh, it is getting complicated. But let's see how it looks in reality. Best Regards, Petr From mboxrd@z Thu Jan 1 00:00:00 1970 From: Petr Mladek Subject: Re: module: save load_info for livepatch modules Date: Thu, 12 Nov 2015 16:03:45 +0100 Message-ID: <20151112150345.GT2599@pathway.suse.cz> References: <1447130755-17383-1-git-send-email-jeyu@redhat.com> <1447130755-17383-3-git-send-email-jeyu@redhat.com> <20151112053228.GD30025@packer-debian-8-amd64.digitalocean.com> <20151112102404.GL4431@pathway.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Miroslav Benes Cc: Jessica Yu , Rusty Russell , Josh Poimboeuf , Seth Jennings , Jiri Kosina , Vojtech Pavlik , linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, live-patching-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-api@vger.kernel.org On Thu 2015-11-12 14:22:28, Miroslav Benes wrote: > On Thu, 12 Nov 2015, Petr Mladek wrote: > > > >Maybe I am missing something but isn't it necessary to call vfree() on > > > >info somewhere in the end? > > > > > > So free_copy() will call vfree(info->hdr), except in livepatch modules > > > we want to keep all the elf section information stored there, so we > > > avoid calling free_copy(), As for the info struct itself, if you look > > > at the init_module and finit_module syscall definitions in > > > kernel/module.c, you will see that info is actually a local function > > > variable, simply passed in to the call to load_module(), and will be > > > automatically deallocated when the syscall returns. :-) No need to > > > explicitly free info. > > > > We still have to free the copied or preserved structures when > > the module is unloaded. > > ...freeing what we allocated. We need to free info->hdr somewhere if not > here and also mod_arch_specific struct where the patch module is removed. > This would unfortunately lead to changes in arch-specific code in > module.c. For example in arch/s390/kernel/module.c there is vfree call on > part of mod_arch_specific in module_finalize. We would call it only if the > flag mentioned above is not set and at the same time we would need to call > it when the patch module is being removed. Sigh, I am afraid that the flag is not enough. IMHO, we need to split the load finalizing functions into two pieces. One will be always called when the module load is finalized. The other part will free the load_info. It will be called either when the load is finalized or when the module is unloaded, depending on if we want to preserve the load_info. Sigh, it is getting complicated. But let's see how it looks in reality. Best Regards, Petr