On 9/14/22 02:43, John Garry wrote:
On 13/09/2022 20:57, Bart Van Assche wrote:
+void scsi_proc_hostdir_add(const struct scsi_host_template *sht)
{
+ struct scsi_proc_entry *e;
+
if (!sht->show_info)
return;
mutex_lock(&global_host_template_mutex);
- if (!sht->present++) {
- sht->proc_dir = proc_mkdir(sht->proc_name, proc_scsi);
- if (!sht->proc_dir)
- printk(KERN_ERR "%s: proc_mkdir failed for %s\n",
- __func__, sht->proc_name);
+ e = __scsi_lookup_proc_entry(sht);
+ if (!e) {
+ e = kzalloc(sizeof(*e), GFP_KERNEL);
So this following scenario is safe, right?
1. host0 calls scsi_proc_hostdir_add() but alloc here fails
2. host1 calls scsi_proc_hostdir_add() but alloc passes, so e->present = 1
3. host0 calls scsi_proc_hostdir_rm(), so e->present goes to zero and we
dealloc the scsi_proc_entry
4. host1 calls scsi_proc_hostdir_rm(), but does not find the
scsi_proc_entry and returns
I suppose the only problem is that we try to free the procfs dir but
host1 still has a procfs entry after 3
I just wonder why we can't make scsi_alloc_alloc() ->
scsi_proc_hostdir_add() error, i.e. return an error code in this
function - it would seem to make things simpler
That sounds like a good idea to me. I will look into this.
+ e->proc_dir = proc_mkdir(sht->proc_name, proc_scsi);
+ if (!e->proc_dir) {
+ printk(KERN_ERR "%s: proc_mkdir failed for %s\n", __func__,
+ sht->proc_name);
+ e->present--;
I don't see why we decrement e->present and then free e
I will remove the code that decrements e->present again.
Thanks,
Bart.
