Hello Andreas Gruenbacher,
The patch b66648ad6dcf: "gfs2: Move inode generation number check
into gfs2_inode_lookup" from Jan 15, 2020, leads to the following
static checker warning:
fs/gfs2/inode.c:227 gfs2_inode_lookup()
warn: passing zero to 'ERR_PTR'
fs/gfs2/inode.c
112 * If @type is DT_UNKNOWN, the inode type is fetched from disk.
113 *
114 * If @blktype is anything other than GFS2_BLKST_FREE (which is used as
a
115 * placeholder because it doesn't otherwise make sense), the on-disk
block type
116 * is verified to be @blktype.
117 *
118 * When @no_formal_ino is non-zero, this function will return
ERR_PTR(-ESTALE)
119 * if it detects that @no_formal_ino doesn't match the actual inode
generation
120 * number. However, it doesn't always know unless @type is DT_UNKNOWN.
121 *
122 * Returns: A VFS inode, or an error
^^^^^^^^^^^^^^^^^^^^^^^^
The comments imply this does not return NULL.
123 */
124
125 struct inode *gfs2_inode_lookup(struct super_block *sb, unsigned int
type,
126 u64 no_addr, u64 no_formal_ino,
127 unsigned int blktype)
128 {
129 struct inode *inode;
130 struct gfs2_inode *ip;
131 struct gfs2_glock *io_gl = NULL;
132 struct gfs2_holder i_gh;
133 int error;
134
135 gfs2_holder_mark_uninitialized(&i_gh);
136 inode = gfs2_iget(sb, no_addr);
137 if (!inode)
138 return ERR_PTR(-ENOMEM);
139
140 ip = GFS2_I(inode);
141
142 if (inode->i_state & I_NEW) {
^^^^^^^^^^^^^^^^^^^^^^
We take this path.
143 struct gfs2_sbd *sdp = GFS2_SB(inode);
144
145 error = gfs2_glock_get(sdp, no_addr, &gfs2_inode_glops,
CREATE, &ip->i_gl);
146 if (unlikely(error))
147 goto fail;
148 flush_delayed_work(&ip->i_gl->gl_work);
149
150 error = gfs2_glock_get(sdp, no_addr, &gfs2_iopen_glops,
CREATE, &io_gl);
151 if (unlikely(error))
152 goto fail;
153
154 if (type == DT_UNKNOWN || blktype != GFS2_BLKST_FREE) {
155 /*
156 * The GL_SKIP flag indicates to skip reading
the inode
157 * block. We read the inode with
gfs2_inode_refresh
158 * after possibly checking the block type.
159 */
160 error = gfs2_glock_nq_init(ip->i_gl,
LM_ST_EXCLUSIVE,
161 GL_SKIP, &i_gh);
162 if (error)
163 goto fail;
164
165 error = -ESTALE;
166 if (no_formal_ino &&
167 gfs2_inode_already_deleted(ip->i_gl,
no_formal_ino))
168 goto fail;
169
170 if (blktype != GFS2_BLKST_FREE) {
171 error = gfs2_check_blk_type(sdp,
no_addr,
172 blktype);
173 if (error)
174 goto fail;
175 }
176 }
177
178 glock_set_object(ip->i_gl, ip);
179 set_bit(GIF_INVALID, &ip->i_flags);
180 error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED,
GL_EXACT, &ip->i_iopen_gh);
181 if (unlikely(error))
182 goto fail;
183 gfs2_cancel_delete_work(ip->i_iopen_gh.gh_gl);
184 glock_set_object(ip->i_iopen_gh.gh_gl, ip);
185 gfs2_glock_put(io_gl);
186 io_gl = NULL;
187
188 /* Lowest possible timestamp; will be overwritten in
gfs2_dinode_in. */
189 inode->i_atime.tv_sec = 1LL << (8 *
sizeof(inode->i_atime.tv_sec) - 1);
190 inode->i_atime.tv_nsec = 0;
191
192 if (type == DT_UNKNOWN) {
193 /* Inode glock must be locked already */
194 error = gfs2_inode_refresh(GFS2_I(inode));
195 if (error)
196 goto fail;
197 } else {
198 ip->i_no_formal_ino = no_formal_ino;
199 inode->i_mode = DT2IF(type);
200 }
201
202 if (gfs2_holder_initialized(&i_gh))
203 gfs2_glock_dq_uninit(&i_gh);
204
205 gfs2_set_iop(inode);
206 }
207
208 if (no_formal_ino && ip->i_no_formal_ino &&
209 no_formal_ino != ip->i_no_formal_ino) {
210 if (inode->i_state & I_NEW)
211 goto fail;
^^^^^^^^^
"error" is zero here.
212 iput(inode);
213 return ERR_PTR(-ESTALE);
214 }
215
216 if (inode->i_state & I_NEW)
217 unlock_new_inode(inode);
218
219 return inode;
220
221 fail:
222 if (io_gl)
223 gfs2_glock_put(io_gl);
224 if (gfs2_holder_initialized(&i_gh))
225 gfs2_glock_dq_uninit(&i_gh);
226 iget_failed(inode);
227 return ERR_PTR(error);
^^^^^
Leading to a NULL return. It doesn't look like the caller checks for
NULL so it might lead to an Oops.
228 }
regards,
dan carpenter
