* [Cluster-devel] GFS2: Move most of the remaining inode.c into ops_inode.c
@ 2015-01-24 19:45 Dan Carpenter
2015-01-24 20:19 ` Steven Whitehouse
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2015-01-24 19:45 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hello Steven Whitehouse,
The [some really old patch], leads to the following static checker
warning:
fs/gfs2/inode.c:203 gfs2_inode_lookup()
error: passing non negative 13 to ERR_PTR
fs/gfs2/inode.c
167 set_bit(GIF_INVALID, &ip->i_flags);
168 error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED, GL_EXACT, &ip->i_iopen_gh);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
It looks like this function can return GLR_TRYFAILED. The caller is
only expecting normal ERR_PTRs so it could cause an oops.
169 if (unlikely(error))
170 goto fail_iopen;
171
172 ip->i_iopen_gh.gh_gl->gl_object = ip;
173 gfs2_glock_put(io_gl);
174 io_gl = NULL;
175
176 if (type == DT_UNKNOWN) {
177 /* Inode glock must be locked already */
178 error = gfs2_inode_refresh(GFS2_I(inode));
179 if (error)
180 goto fail_refresh;
181 } else {
182 inode->i_mode = DT2IF(type);
183 }
184
185 gfs2_set_iop(inode);
186 unlock_new_inode(inode);
187 }
188
189 return inode;
190
191 fail_refresh:
192 ip->i_iopen_gh.gh_flags |= GL_NOCACHE;
193 ip->i_iopen_gh.gh_gl->gl_object = NULL;
194 gfs2_glock_dq_uninit(&ip->i_iopen_gh);
195 fail_iopen:
196 if (io_gl)
197 gfs2_glock_put(io_gl);
198 fail_put:
199 ip->i_gl->gl_object = NULL;
200 gfs2_glock_put(ip->i_gl);
201 fail:
202 iget_failed(inode);
203 return ERR_PTR(error);
204 }
Related:
fs/gfs2/inode.c:203 gfs2_inode_lookup() error: passing non negative 13 to ERR_PTR
fs/gfs2/inode.c:218 gfs2_lookup_by_inum() error: passing non negative 13 to ERR_PTR
fs/gfs2/inode.c:243 gfs2_lookup_by_inum() error: passing non negative 13 to ERR_PTR
fs/gfs2/inode.c:306 gfs2_lookupi() error: passing non negative 13 to ERR_PTR
fs/gfs2/inode.c:324 gfs2_lookupi() error: passing non negative 13 to ERR_PTR
fs/gfs2/inode.c:852 __gfs2_lookup() error: passing non negative 13 to ERR_PTR
fs/gfs2/inode.c:1567 gfs2_follow_link() error: passing non negative 13 to ERR_PTR
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* [Cluster-devel] GFS2: Move most of the remaining inode.c into ops_inode.c
2015-01-24 19:45 [Cluster-devel] GFS2: Move most of the remaining inode.c into ops_inode.c Dan Carpenter
@ 2015-01-24 20:19 ` Steven Whitehouse
2015-01-26 7:57 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Steven Whitehouse @ 2015-01-24 20:19 UTC (permalink / raw)
To: cluster-devel.redhat.com
Hi,
On 24/01/15 19:45, Dan Carpenter wrote:
> Hello Steven Whitehouse,
>
> The [some really old patch], leads to the following static checker
> warning:
>
> fs/gfs2/inode.c:203 gfs2_inode_lookup()
> error: passing non negative 13 to ERR_PTR
>
> fs/gfs2/inode.c
> 167 set_bit(GIF_INVALID, &ip->i_flags);
> 168 error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED, GL_EXACT, &ip->i_iopen_gh);
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> It looks like this function can return GLR_TRYFAILED. The caller is
> only expecting normal ERR_PTRs so it could cause an oops.
This does not request a try lock, so it should never return the
GLR_TRYFAILED value. I think the checker is perhaps not following the
code well enough to figure that out?
Steve.
> 169 if (unlikely(error))
> 170 goto fail_iopen;
> 171
> 172 ip->i_iopen_gh.gh_gl->gl_object = ip;
> 173 gfs2_glock_put(io_gl);
> 174 io_gl = NULL;
> 175
> 176 if (type == DT_UNKNOWN) {
> 177 /* Inode glock must be locked already */
> 178 error = gfs2_inode_refresh(GFS2_I(inode));
> 179 if (error)
> 180 goto fail_refresh;
> 181 } else {
> 182 inode->i_mode = DT2IF(type);
> 183 }
> 184
> 185 gfs2_set_iop(inode);
> 186 unlock_new_inode(inode);
> 187 }
> 188
> 189 return inode;
> 190
> 191 fail_refresh:
> 192 ip->i_iopen_gh.gh_flags |= GL_NOCACHE;
> 193 ip->i_iopen_gh.gh_gl->gl_object = NULL;
> 194 gfs2_glock_dq_uninit(&ip->i_iopen_gh);
> 195 fail_iopen:
> 196 if (io_gl)
> 197 gfs2_glock_put(io_gl);
> 198 fail_put:
> 199 ip->i_gl->gl_object = NULL;
> 200 gfs2_glock_put(ip->i_gl);
> 201 fail:
> 202 iget_failed(inode);
> 203 return ERR_PTR(error);
> 204 }
>
> Related:
> fs/gfs2/inode.c:203 gfs2_inode_lookup() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:218 gfs2_lookup_by_inum() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:243 gfs2_lookup_by_inum() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:306 gfs2_lookupi() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:324 gfs2_lookupi() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:852 __gfs2_lookup() error: passing non negative 13 to ERR_PTR
> fs/gfs2/inode.c:1567 gfs2_follow_link() error: passing non negative 13 to ERR_PTR
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* [Cluster-devel] GFS2: Move most of the remaining inode.c into ops_inode.c
2015-01-24 20:19 ` Steven Whitehouse
@ 2015-01-26 7:57 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2015-01-26 7:57 UTC (permalink / raw)
To: cluster-devel.redhat.com
On Sat, Jan 24, 2015 at 08:19:47PM +0000, Steven Whitehouse wrote:
> Hi,
>
> On 24/01/15 19:45, Dan Carpenter wrote:
> >Hello Steven Whitehouse,
> >
> >The [some really old patch], leads to the following static checker
> >warning:
> >
> > fs/gfs2/inode.c:203 gfs2_inode_lookup()
> > error: passing non negative 13 to ERR_PTR
> >
> >fs/gfs2/inode.c
> > 167 set_bit(GIF_INVALID, &ip->i_flags);
> > 168 error = gfs2_glock_nq_init(io_gl, LM_ST_SHARED, GL_EXACT, &ip->i_iopen_gh);
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >It looks like this function can return GLR_TRYFAILED. The caller is
> >only expecting normal ERR_PTRs so it could cause an oops.
> This does not request a try lock, so it should never return the
> GLR_TRYFAILED value. I think the checker is perhaps not following
> the code well enough to figure that out?
Ooo.. Yes. That's too tricky for the static checker. Thanks for
looking into this.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-01-26 7:57 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-01-24 19:45 [Cluster-devel] GFS2: Move most of the remaining inode.c into ops_inode.c Dan Carpenter
2015-01-24 20:19 ` Steven Whitehouse
2015-01-26 7:57 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.