On 4/5/19 3:08 AM, Dan Carpenter wrote:
Hello Chengguang Xu,
The patch 4b0be5726032: "chardev: code cleanup for
__register_chrdev_region()" from Feb 15, 2019, leads to the following
static checker warning:
fs/char_dev.c:167 __register_chrdev_region()
error: passing non negative 511 to ERR_PTR
Hi Dan,
Thanks for your report, I would like to know how did you trigger this?
IIUC, in the case of allocating dynamic major will not fail from minor
overlap check.
Thanks,
Chengguang.
fs/char_dev.c
96 static struct char_device_struct *
97 __register_chrdev_region(unsigned int major, unsigned int baseminor,
98 int minorct, const char *name)
99 {
100 struct char_device_struct *cd, *curr, *prev = NULL;
101 int ret = -EBUSY;
102 int i;
103
104 if (major >= CHRDEV_MAJOR_MAX) {
105 pr_err("CHRDEV \"%s\" major requested (%u) is greater than the maximum (%u)\n",
106 name, major, CHRDEV_MAJOR_MAX-1);
107 return ERR_PTR(-EINVAL);
108 }
109
110 if (minorct > MINORMASK + 1 - baseminor) {
111 pr_err("CHRDEV \"%s\" minor range requested (%u-%u) is out of range of maximum range (%u-%u) for a single major\n",
112 name, baseminor, baseminor + minorct - 1, 0, MINORMASK);
113 return ERR_PTR(-EINVAL);
114 }
115
116 cd = kzalloc(sizeof(struct char_device_struct), GFP_KERNEL);
117 if (cd == NULL)
118 return ERR_PTR(-ENOMEM);
119
120 mutex_lock(&chrdevs_lock);
121
122 if (major == 0) {
123 ret = find_dynamic_major();
124 if (ret < 0) {
125 pr_err("CHRDEV \"%s\" dynamic allocation region is full\n",
126 name);
127 goto out;
128 }
129 major = ret;
^^^^^^^^^^^
"ret" is a major here.
130 }
131
132 i = major_to_index(major);
133 for (curr = chrdevs[i]; curr; prev = curr, curr = curr->next) {
134 if (curr->major < major)
135 continue;
136
137 if (curr->major > major)
138 break;
139
140 if (curr->baseminor + curr->minorct <= baseminor)
141 continue;
142
143 if (curr->baseminor >= baseminor + minorct)
144 break;
145
146 goto out;
I don't completely understand how this loop works, but I guess we should
set "ret = -EBUSY;" before the goto out.
147 }
148
149 cd->major = major;
150 cd->baseminor = baseminor;
151 cd->minorct = minorct;
152 strlcpy(cd->name, name, sizeof(cd->name));
153
154 if (!prev) {
155 cd->next = curr;
156 chrdevs[i] = cd;
157 } else {
158 cd->next = prev->next;
159 prev->next = cd;
160 }
161
162 mutex_unlock(&chrdevs_lock);
163 return cd;
164 out:
165 mutex_unlock(&chrdevs_lock);
166 kfree(cd);
167 return ERR_PTR(ret);
^^^
Otherwise it leads to an Oops in the caller.
168 }
regards,
dan carpenter
