Message-ID: <7590df81-3bdd-b04e-7b4e-797b45fabded@alu.unizg.hr>
Date:   Thu, 9 Mar 2023 09:14:08 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: INFO: BUG: kobject: integrity does not have a release function
Content-Language: en-US
To:     Hillf Danton <hdanton@sina.com>
Cc:     linux-kernel@vger.kernel.org,
        Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
        Paul Moore <paul@paul-moore.com>,
        Andy Shevchenko <andriy.shevchenko@intel.com>
References: <20230309021556.2203-1-hdanton@sina.com>
From:   Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
In-Reply-To: <20230309021556.2203-1-hdanton@sina.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

On 9.3.2023. 3:15, Hillf Danton wrote:
> On 8 Mar 2023 15:08:21 +0100 Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
>>
>> Please see the bug reports from the kernel log:
>>
>> https://domac.alu.hr/~mtodorov/linux/bugreports/integrity/20230308_123748.jpg
>>
>> https://domac.alu.hr/~mtodorov/linux/bugreports/integrity/20230308_123752.jpg
>>
>> The kernel is Linux 6.2.0-mg-andy-devres-12485-gf3a2439f20d9-dirty x86_64
>> on a LENOVO_MT_10TX_BU_Lenovo_FM_V530S-07ICB running AlmaLinux 8.7.
> 
> Only in case no support from your linux vendor, take a look at the debug/fix
> diffs below.
>>
>> I was unable to reproduce on the other Lenovo laptop box, for the kernel
>> refused to boot, unable to find root drive on NVMe (other kernels w/o
>> CONFIG_DEBUG_KOBJECT=y run smoothly).
>>
>> Config used is:
>>
>> https://domac.alu.hr/~mtodorov/linux/bugreports/integrity/config-6.2.0-mg-andy-devres-12485-gf3a2439f20d9-dirty
>>
>> As I already said to Andy, this might not be a critical bug, for it happens
>> only at shutdown AFAICS. However, it can be a sign of some more serious problem
>> in the code. :-/
>>
>> Hope this helps.
>>
>> Regards,
>> Mirsad
>>
>> -- 
>> Mirsad Goran Todorovac
>> Sistem inženjer
>> Grafički fakultet | Akademija likovnih umjetnosti
>> Sveučilište u Zagrebu
>>
>> System engineer
>> Faculty of Graphic Arts | Academy of Fine Arts
>> University of Zagreb, Republic of Croatia
> 
> Add a dummy release callback to the integrity ktype in order to
> quiesce kobj warning.
> 
> --- 6.2/block/blk-integrity.c
> +++ b/block/blk-integrity.c
> @@ -356,7 +356,11 @@ static const struct sysfs_ops integrity_
>   	.store	= &integrity_attr_store,
>   };
>   
> +static void blk_integrity_kobj_release(struct kobject *kobj)
> +{
> +}
>   static const struct kobj_type integrity_ktype = {
> +	.release = blk_integrity_kobj_release,
>   	.default_groups = integrity_groups,
>   	.sysfs_ops	= &integrity_ops,
>   };
> --
> 
> Add debug info to catch invalid kobj type.
> 
> --- 6.2/lib/kobject.c
> +++ b/lib/kobject.c
> @@ -443,6 +443,8 @@ int kobject_init_and_add(struct kobject
>   	va_list args;
>   	int retval;
>   
> +	if (WARN_ON(!ktype || !ktype->release))
> +		return -EINVAL;
>   	kobject_init(kobj, ktype);
>   
>   	va_start(args, fmt);
> --

Hi, Hillf, thank you very much for the patch.

However, I have seen another inconsistency here:

security/integrity/iint.c:
175 static int __init integrity_iintcache_init(void)
176 {
177         iint_cache =
178             kmem_cache_create("iint_cache", sizeof(struct integrity_iint_cache),
179                               0, SLAB_PANIC, init_once);
180         return 0;
181 }
182 DEFINE_LSM(integrity) = {
183         .name = "integrity",
184         .init = integrity_iintcache_init,
185 };

There is no call to kmem_cache_destroy(iint_cache) in entire security/integrity subtree?

Is that intentional?

I know there is little logic in releasing resources at shutdown, but it seems like the
right thing to do ... :-/

Thank you very much, again.

Regards,
Mirsad

-- 
Mirsad Todorovac
System engineer
Faculty of Graphic Arts | Academy of Fine Arts
University of Zagreb
Republic of Croatia, the European Union

Sistem inženjer
Grafički fakultet | Akademija likovnih umjetnosti
Sveučilište u Zagrebu